Merge pull request #1682 from WordPress-Coding-Standards/feature/validatedsanitizedinput-improve-array-comparison-handling

ValidatedSanitizedInput: treat array-value comparison functions same as other comparisons

Author: GaryJones

WordPress/Sniff.php

@@ -256,7 +256,6 @@ abstract class Sniff implements PHPCS_Sniff {
 		'filter_input'               => true,
 		'filter_var'                 => true,
 		'hash_equals'                => true,
-		'in_array'                   => true,
 		'is_email'                   => true,
 		'number_format'              => true,
 		'sanitize_bookmark_field'    => true,
@@ -380,6 +379,22 @@ abstract class Sniff implements PHPCS_Sniff {
 		'map_deep'  => 2,
 	);
 
+	/**
+	 * Array functions to compare a $needle to a predefined set of values.
+	 *
+	 * If the value is set to an integer, the function needs to have at least that
+	 * many parameters for it to be considered as a comparison.
+	 *
+	 * @since 2.1.0
+	 *
+	 * @var array <string function name> => <true|int>
+	 */
+	protected $arrayCompareFunctions = array(
+		'in_array'     => true,
+		'array_search' => true,
+		'array_keys'   => 2,
+	);
+
 	/**
 	 * Functions that format strings.
 	 *
@@ -2273,6 +2288,34 @@ protected function is_comparison( $stackPtr ) {
 		return false;
 	}
 
+	/**
+	 * Check if a token is inside of an array-value comparison function.
+	 *
+	 * @since 2.1.0
+	 *
+	 * @param int $stackPtr The index of the token in the stack.
+	 *
+	 * @return bool Whether the token is (part of) a parameter to an
+	 *              array-value comparison function.
+	 */
+	protected function is_in_array_comparison( $stackPtr ) {
+		$function_ptr = $this->is_in_function_call( $stackPtr, $this->arrayCompareFunctions, true, true );
+		if ( false === $function_ptr ) {
+			return false;
+		}
+
+		$function_name = $this->tokens[ $function_ptr ]['content'];
+		if ( true === $this->arrayCompareFunctions[ $function_name ] ) {
+			return true;
+		}
+
+		if ( $this->get_function_call_parameter_count( $function_ptr ) >= $this->arrayCompareFunctions[ $function_name ] ) {
+			return true;
+		}
+
+		return false;
+	}
+
 	/**
 	 * Check what type of 'use' statement a token is part of.
 	 *

WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php

@@ -155,6 +155,11 @@ function ( $symbol ) {
 			return;
 		}
 
+		// If this is a comparison using the array comparison functions, sanitization isn't needed.
+		if ( $this->is_in_array_comparison( $stackPtr ) ) {
+			return;
+		}
+
 		$this->mergeFunctionLists();
 
 		// Now look for sanitizing functions.

WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.inc

@@ -276,3 +276,16 @@ function test_additional_array_walking_functions() {
 	$sane = map_deep( wp_unslash( $_GET['test'] ), 'sanitize_text_field' ); // Ok.
 	$sane = map_deep( wp_unslash( $_GET['test'] ), 'foo' ); // Bad.
 }
+
+function test_recognize_array_comparison_functions_as_such() {
+	if ( ! isset( $_POST['form_fields'] ) ) {
+		return;
+	}
+
+	if ( isset( $_REQUEST['plugin_status'] ) && in_array( $_REQUEST['plugin_status'], array( 'install', 'update', 'activate' ), true ) ) {} // OK.
+	if ( isset( $_REQUEST['plugin_state'] ) && in_array( $_REQUEST['plugin_state'], $states, true ) ) {} // OK.
+	if ( in_array( 'my_form_hidden_field_value', $_POST['form_fields'], true ) ) {} // OK.
+	if ( array_search( $_POST['form_fields'], 'my_form_hidden_field_value' ) ) {} // OK.
+	if ( array_keys( $_POST['form_fields'], 'my_form_hidden_field_value', true ) ) {} // OK.
+	if ( array_keys( $_POST['form_fields'] ) ) {} // Bad x2.
+}

WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.php

@@ -67,6 +67,7 @@ public function getErrorList() {
 			257 => 1,
 			266 => 1,
 			277 => 1,
+			290 => 2,
 		);
 	}