diff --git a/WordPress/Helpers/ContextHelper.php b/WordPress/Helpers/ContextHelper.php index 8bc1a91ace..757479e46d 100644 --- a/WordPress/Helpers/ContextHelper.php +++ b/WordPress/Helpers/ContextHelper.php @@ -91,6 +91,27 @@ final class ContextHelper { 'key_exists' => true, // Alias. ); + /** + * Array functions to compare a $needle to a predefined set of values. + * + * If the value is set to an array, the parameter specified in the array is + * required for the function call to be considered as a comparison. + * + * @since 2.1.0 + * @since 3.0.0 - Moved from the Sniff class to this class. + * - The property visibility was changed from `protected` to `private static`. + * + * @var array => + */ + private static $arrayCompareFunctions = array( + 'in_array' => true, + 'array_search' => true, + 'array_keys' => array( + 'position' => 2, + 'name' => 'filter_value', + ), + ); + /** * Check if a particular token acts - statically or non-statically - on an object. * @@ -303,4 +324,38 @@ public static function is_safe_casted( File $phpcsFile, $stackPtr ) { return isset( self::$safe_casts[ $tokens[ $prev ]['code'] ] ); } + + /** + * Check if a token is inside of an array-value comparison function. + * + * @since 2.1.0 + * @since 3.0.0 - Moved from the Sniff class to this class. + * - The method visibility was changed from `protected` to `public static`. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @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. + */ + public static function is_in_array_comparison( File $phpcsFile, $stackPtr ) { + $function_ptr = self::is_in_function_call( $phpcsFile, $stackPtr, self::$arrayCompareFunctions, true, true ); + if ( false === $function_ptr ) { + return false; + } + + $tokens = $phpcsFile->getTokens(); + $function_name = strtolower( $tokens[ $function_ptr ]['content'] ); + if ( true === self::$arrayCompareFunctions[ $function_name ] ) { + return true; + } + + $target_param = self::$arrayCompareFunctions[ $function_name ]; + $found_param = PassedParameters::getParameter( $phpcsFile, $function_ptr, $target_param['position'], $target_param['name'] ); + if ( false !== $found_param ) { + return true; + } + + return false; + } } diff --git a/WordPress/Sniff.php b/WordPress/Sniff.php index 8190c33c3a..8416a6516d 100644 --- a/WordPress/Sniff.php +++ b/WordPress/Sniff.php @@ -268,22 +268,6 @@ 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 => - */ - protected $arrayCompareFunctions = array( - 'in_array' => true, - 'array_search' => true, - 'array_keys' => 2, - ); - /** * Functions that format strings. * @@ -838,34 +822,6 @@ protected function is_validated( $stackPtr, $array_keys = array(), $in_condition 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 = ContextHelper::is_in_function_call( $this->phpcsFile, $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 ( PassedParameters::getParameterCount( $this->phpcsFile, $function_ptr ) >= $this->arrayCompareFunctions[ $function_name ] ) { - return true; - } - - return false; - } - /** * Determine whether an arbitrary T_STRING token is the use of a global constant. * diff --git a/WordPress/Sniffs/Security/NonceVerificationSniff.php b/WordPress/Sniffs/Security/NonceVerificationSniff.php index 3e551dc30b..ef33fb3fb6 100644 --- a/WordPress/Sniffs/Security/NonceVerificationSniff.php +++ b/WordPress/Sniffs/Security/NonceVerificationSniff.php @@ -203,7 +203,7 @@ private function has_nonce_check( $stackPtr ) { if ( ContextHelper::is_in_isset_or_empty( $this->phpcsFile, $stackPtr ) || ContextHelper::is_in_type_test( $this->phpcsFile, $stackPtr ) || VariableHelper::is_comparison( $this->phpcsFile, $stackPtr ) - || $this->is_in_array_comparison( $stackPtr ) + || ContextHelper::is_in_array_comparison( $this->phpcsFile, $stackPtr ) || ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $this->unslashingFunctions ) !== false || $this->is_only_sanitized( $stackPtr ) ) { diff --git a/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php b/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php index c34a84f8eb..f6994bb7e4 100644 --- a/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php +++ b/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php @@ -188,7 +188,7 @@ function( $embed ) { } // If this is a comparison using the array comparison functions, sanitization isn't needed. - if ( $this->is_in_array_comparison( $stackPtr ) ) { + if ( ContextHelper::is_in_array_comparison( $this->phpcsFile, $stackPtr ) ) { return; } diff --git a/WordPress/Tests/Security/NonceVerificationUnitTest.inc b/WordPress/Tests/Security/NonceVerificationUnitTest.inc index af7734fa39..a7da9675dd 100644 --- a/WordPress/Tests/Security/NonceVerificationUnitTest.inc +++ b/WordPress/Tests/Security/NonceVerificationUnitTest.inc @@ -362,3 +362,32 @@ function allow_in_array_key_exists_before_noncecheck_with_named_params() { wp_verify_nonce( 'some_action' ); } + +// Tests specifically for the ContextHelper::is_in_array_comparison(). +function allow_for_array_comparison_in_condition() { + if ( Array_Keys( $_GET['actions'], 'my_action', true ) ) { // OK. + check_admin_referer( 'foo' ); + foo(); + } +} + +function disallow_for_non_array_comparison_in_condition() { + if ( array_keys( $_GET['actions'] ) ) { // Bad. + check_admin_referer( 'foo' ); + foo(); + } +} + +function allow_for_array_comparison_in_condition_with_named_params() { + if ( array_keys( filter_value: 'my_action', array: $_GET['actions'], strict: true, ) ) { // OK. + check_admin_referer( 'foo' ); + foo(); + } +} + +function disallow_for_non_array_comparison_in_condition_with_named_params() { + if ( array_keys( strict: true, array: $_GET['actions'], ) ) { // Bad, missing $filter_value param. Invalid function call, but not our concern. + check_admin_referer( 'foo' ); + foo(); + } +} diff --git a/WordPress/Tests/Security/NonceVerificationUnitTest.php b/WordPress/Tests/Security/NonceVerificationUnitTest.php index 7e34eaec7e..edcc509cb0 100644 --- a/WordPress/Tests/Security/NonceVerificationUnitTest.php +++ b/WordPress/Tests/Security/NonceVerificationUnitTest.php @@ -23,6 +23,7 @@ * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_in_function_call * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_in_type_test * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_in_isset_or_empty + * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_in_array_comparison * @covers \WordPressCS\WordPress\Sniffs\Security\NonceVerificationSniff */ final class NonceVerificationUnitTest extends AbstractSniffUnitTest { @@ -74,7 +75,9 @@ public function getErrorList() { * @return array => */ public function getWarningList() { - return array(); + return array( + 375 => 1, + 389 => 1, + ); } - }