diff --git a/WordPress/Helpers/ContextHelper.php b/WordPress/Helpers/ContextHelper.php index 19994c422c..8ddbb29c4e 100644 --- a/WordPress/Helpers/ContextHelper.php +++ b/WordPress/Helpers/ContextHelper.php @@ -81,4 +81,86 @@ public static function is_token_namespaced( File $phpcsFile, $stackPtr ) { return true; } + + /** + * Check if a token is (part of) a parameter for a function call to a select list of functions. + * + * This is useful, for instance, when trying to determine the context a variable is used in. + * + * For example: this function could be used to determine if the variable `$foo` is used + * in a global function call to the function `is_foo()`. + * In that case, a call to this function would return the stackPtr to the T_STRING `is_foo` + * for code like: `is_foo( $foo, 'some_other_param' )`, while it would return `false` for + * the following code `is_bar( $foo, 'some_other_param' )`. + * + * @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. + * @param array $valid_functions List of valid function names. + * Note: The keys to this array should be the function names + * in lowercase. Values are irrelevant. + * @param bool $global_function Optional. Whether to make sure that the function call is + * to a global function. If `false`, calls to methods, be it static + * `Class::method()` or via an object `$obj->method()`, and + * namespaced function calls, like `MyNS\function_name()` will + * also be accepted. + * Defaults to `true`. + * @param bool $allow_nested Optional. Whether to allow for nested function calls within the + * call to this function. + * I.e. when checking whether a token is within a function call + * to `strtolower()`, whether to accept `strtolower( trim( $var ) )` + * or only `strtolower( $var )`. + * Defaults to `false`. + * + * @return int|bool Stack pointer to the function call T_STRING token or false otherwise. + */ + public static function is_in_function_call( File $phpcsFile, $stackPtr, array $valid_functions, $global_function = true, $allow_nested = false ) { + $tokens = $phpcsFile->getTokens(); + if ( ! isset( $tokens[ $stackPtr ]['nested_parenthesis'] ) ) { + return false; + } + + $nested_parenthesis = $tokens[ $stackPtr ]['nested_parenthesis']; + if ( false === $allow_nested ) { + $nested_parenthesis = array_reverse( $nested_parenthesis, true ); + } + + foreach ( $nested_parenthesis as $open => $close ) { + $prev_non_empty = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $open - 1 ), null, true ); + if ( false === $prev_non_empty || \T_STRING !== $tokens[ $prev_non_empty ]['code'] ) { + continue; + } + + if ( isset( $valid_functions[ strtolower( $tokens[ $prev_non_empty ]['content'] ) ] ) === false ) { + if ( false === $allow_nested ) { + // Function call encountered, but not to one of the allowed functions. + return false; + } + + continue; + } + + if ( false === $global_function ) { + return $prev_non_empty; + } + + /* + * Now, make sure it is a global function. + */ + if ( self::has_object_operator_before( $phpcsFile, $prev_non_empty ) === true ) { + continue; + } + + if ( self::is_token_namespaced( $phpcsFile, $prev_non_empty ) === true ) { + continue; + } + + return $prev_non_empty; + } + + return false; + } } diff --git a/WordPress/Sniff.php b/WordPress/Sniff.php index fbd654b9c2..e1a8ad89d3 100644 --- a/WordPress/Sniff.php +++ b/WordPress/Sniff.php @@ -497,7 +497,7 @@ protected function is_in_isset_or_empty( $stackPtr ) { 'key_exists' => true, // Alias. ); - $functionPtr = $this->is_in_function_call( $stackPtr, $valid_functions ); + $functionPtr = ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $valid_functions ); if ( false !== $functionPtr ) { $second_param = PassedParameters::getParameter( $this->phpcsFile, $functionPtr, 2 ); if ( $stackPtr >= $second_param['start'] && $stackPtr <= $second_param['end'] ) { @@ -508,85 +508,6 @@ protected function is_in_isset_or_empty( $stackPtr ) { return false; } - /** - * Check if a token is (part of) a parameter for a function call to a select list of functions. - * - * This is useful, for instance, when trying to determine the context a variable is used in. - * - * For example: this function could be used to determine if the variable `$foo` is used - * in a global function call to the function `is_foo()`. - * In that case, a call to this function would return the stackPtr to the T_STRING `is_foo` - * for code like: `is_foo( $foo, 'some_other_param' )`, while it would return `false` for - * the following code `is_bar( $foo, 'some_other_param' )`. - * - * @since 2.1.0 - * - * @param int $stackPtr The index of the token in the stack. - * @param array $valid_functions List of valid function names. - * Note: The keys to this array should be the function names - * in lowercase. Values are irrelevant. - * @param bool $global_function Optional. Whether to make sure that the function call is - * to a global function. If `false`, calls to methods, be it static - * `Class::method()` or via an object `$obj->method()`, and - * namespaced function calls, like `MyNS\function_name()` will - * also be accepted. - * Defaults to `true`. - * @param bool $allow_nested Optional. Whether to allow for nested function calls within the - * call to this function. - * I.e. when checking whether a token is within a function call - * to `strtolower()`, whether to accept `strtolower( trim( $var ) )` - * or only `strtolower( $var )`. - * Defaults to `false`. - * - * @return int|bool Stack pointer to the function call T_STRING token or false otherwise. - */ - protected function is_in_function_call( $stackPtr, $valid_functions, $global_function = true, $allow_nested = false ) { - if ( ! isset( $this->tokens[ $stackPtr ]['nested_parenthesis'] ) ) { - return false; - } - - $nested_parenthesis = $this->tokens[ $stackPtr ]['nested_parenthesis']; - if ( false === $allow_nested ) { - $nested_parenthesis = array_reverse( $nested_parenthesis, true ); - } - - foreach ( $nested_parenthesis as $open => $close ) { - - $prev_non_empty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $open - 1 ), null, true, null, true ); - if ( false === $prev_non_empty || \T_STRING !== $this->tokens[ $prev_non_empty ]['code'] ) { - continue; - } - - if ( isset( $valid_functions[ strtolower( $this->tokens[ $prev_non_empty ]['content'] ) ] ) === false ) { - if ( false === $allow_nested ) { - // Function call encountered, but not to one of the allowed functions. - return false; - } - - continue; - } - - if ( false === $global_function ) { - return $prev_non_empty; - } - - /* - * Now, make sure it is a global function. - */ - if ( ContextHelper::has_object_operator_before( $this->phpcsFile, $prev_non_empty ) === true ) { - continue; - } - - if ( ContextHelper::is_token_namespaced( $this->phpcsFile, $prev_non_empty ) === true ) { - continue; - } - - return $prev_non_empty; - } - - return false; - } - /** * Check if a token is inside of an is_...() statement. * @@ -602,7 +523,7 @@ protected function is_in_type_test( $stackPtr ) { * The return can never be `0` as there will always be a PHP open tag before the * function call. */ - return (bool) $this->is_in_function_call( $stackPtr, $this->typeTestFunctions ); + return (bool) ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $this->typeTestFunctions ); } /** @@ -708,7 +629,7 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) { $valid_functions += $this->unslashingFunctions; $valid_functions += $this->arrayWalkingFunctions; - $functionPtr = $this->is_in_function_call( $stackPtr, $valid_functions ); + $functionPtr = ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $valid_functions ); // If this isn't a call to one of the valid functions, it sure isn't a sanitizing function. if ( false === $functionPtr ) { @@ -730,7 +651,7 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) { $valid_functions = array_diff_key( $valid_functions, $this->unslashingFunctions ); // Check is any of the remaining (sanitizing) functions is used. - $higherFunctionPtr = $this->is_in_function_call( $functionPtr, $valid_functions ); + $higherFunctionPtr = ContextHelper::is_in_function_call( $this->phpcsFile, $functionPtr, $valid_functions ); // If there is no other valid function being used, this value is unsanitized. if ( false === $higherFunctionPtr ) { @@ -1067,7 +988,7 @@ protected function is_validated( $stackPtr, $array_keys = array(), $in_condition * array-value comparison function. */ protected function is_in_array_comparison( $stackPtr ) { - $function_ptr = $this->is_in_function_call( $stackPtr, $this->arrayCompareFunctions, true, true ); + $function_ptr = ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $this->arrayCompareFunctions, true, true ); if ( false === $function_ptr ) { return false; } diff --git a/WordPress/Sniffs/Security/NonceVerificationSniff.php b/WordPress/Sniffs/Security/NonceVerificationSniff.php index 957e1eebdc..e7a6dc4114 100644 --- a/WordPress/Sniffs/Security/NonceVerificationSniff.php +++ b/WordPress/Sniffs/Security/NonceVerificationSniff.php @@ -204,7 +204,7 @@ private function has_nonce_check( $stackPtr ) { || $this->is_in_type_test( $stackPtr ) || VariableHelper::is_comparison( $this->phpcsFile, $stackPtr ) || $this->is_in_array_comparison( $stackPtr ) - || $this->is_in_function_call( $stackPtr, $this->unslashingFunctions ) !== false + || ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $this->unslashingFunctions ) !== false || $this->is_only_sanitized( $stackPtr ) ) { $allow_nonce_after = true; diff --git a/WordPress/Sniffs/WP/CapitalPDangitSniff.php b/WordPress/Sniffs/WP/CapitalPDangitSniff.php index 27a38bebd3..678ea7dfcd 100644 --- a/WordPress/Sniffs/WP/CapitalPDangitSniff.php +++ b/WordPress/Sniffs/WP/CapitalPDangitSniff.php @@ -13,6 +13,7 @@ use PHPCSUtils\Tokens\Collections; use PHPCSUtils\Utils\ObjectDeclarations; use PHPCSUtils\Utils\Namespaces; +use WordPressCS\WordPress\Helpers\ContextHelper; use WordPressCS\WordPress\Sniff; /** @@ -209,7 +210,7 @@ public function process_token( $stackPtr ) { } // Ignore constant declarations via define(). - if ( $this->is_in_function_call( $stackPtr, array( 'define' => true ), true, true ) ) { + if ( ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, array( 'define' => true ), true, true ) ) { return; } diff --git a/WordPress/Sniffs/WP/CronIntervalSniff.php b/WordPress/Sniffs/WP/CronIntervalSniff.php index 84d1792b49..055eccf02a 100644 --- a/WordPress/Sniffs/WP/CronIntervalSniff.php +++ b/WordPress/Sniffs/WP/CronIntervalSniff.php @@ -9,7 +9,6 @@ namespace WordPressCS\WordPress\Sniffs\WP; -use WordPressCS\WordPress\Sniff; use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Tokens\Collections; use PHPCSUtils\Utils\Arrays; @@ -17,6 +16,8 @@ use PHPCSUtils\Utils\Numbers; use PHPCSUtils\Utils\PassedParameters; use PHPCSUtils\Utils\TextStrings; +use WordPressCS\WordPress\Helpers\ContextHelper; +use WordPressCS\WordPress\Sniff; /** * Flag cron schedules less than 15 minutes. @@ -94,7 +95,7 @@ public function process_token( $stackPtr ) { } // Check if the text was found within a function call to add_filter(). - $functionPtr = $this->is_in_function_call( $stackPtr, $this->valid_functions ); + $functionPtr = ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, $this->valid_functions ); if ( false === $functionPtr ) { return; } diff --git a/WordPress/Tests/Security/NonceVerificationUnitTest.inc b/WordPress/Tests/Security/NonceVerificationUnitTest.inc index e3a12f28e2..c4e49c974d 100644 --- a/WordPress/Tests/Security/NonceVerificationUnitTest.inc +++ b/WordPress/Tests/Security/NonceVerificationUnitTest.inc @@ -301,3 +301,15 @@ function function_containing_nested_closure() { }; } +// Tests specifically for the ContextHelper::is_in_function_call(). +function disallow_custom_unslash_before_noncecheck_via_method() { + $var = MyClass::stripslashes_from_strings_only( $_POST['foo'] ); // Bad. + wp_verify_nonce( $var ); + echo $var; +} + +function disallow_custom_unslash_before_noncecheck_via_namespaced_function() { + $var = MyNamespace\stripslashes_from_strings_only( $_POST['foo'] ); // Bad. + wp_verify_nonce( $var ); + echo $var; +} diff --git a/WordPress/Tests/Security/NonceVerificationUnitTest.php b/WordPress/Tests/Security/NonceVerificationUnitTest.php index 2fb94c8da5..3cf402baee 100644 --- a/WordPress/Tests/Security/NonceVerificationUnitTest.php +++ b/WordPress/Tests/Security/NonceVerificationUnitTest.php @@ -20,6 +20,7 @@ * @since 0.13.0 Class name changed: this class is now namespaced. * @since 1.0.0 This sniff has been moved from the `CSRF` category to the `Security` category. * + * @covers \WordPressCS\WordPress\Helpers\ContextHelper::is_in_function_call * @covers \WordPressCS\WordPress\Sniffs\Security\NonceVerificationSniff */ final class NonceVerificationUnitTest extends AbstractSniffUnitTest { @@ -56,6 +57,8 @@ public function getErrorList() { 202 => 1, 252 => 1, 269 => 1, + 306 => 1, + 312 => 1, ); }