Skip to content

Commit

Permalink
Merge pull request #2229 from WordPress/feature/move-isinfunctioncall…
Browse files Browse the repository at this point in the history 
…-to-helper-class

Move `is_in_function_call()` utility method to dedicated `ContextHelper`
  • Loading branch information
@dingo-d
dingo-d committed Apr 20, 2023
2 parents 6757824 + 6d6106f commit a25d821
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 88 deletions.
82 changes: 82 additions & 0 deletions WordPress/Helpers/ContextHelper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
89 changes: 5 additions & 84 deletions WordPress/Sniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'] ) {
Expand All @@ -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.
*
Expand All @@ -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 );
}

/**
Expand Down Expand Up @@ -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 ) {
Expand All @@ -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 ) {
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Sniffs/Security/NonceVerificationSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion WordPress/Sniffs/WP/CapitalPDangitSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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;
}

Expand Down
5 changes: 3 additions & 2 deletions WordPress/Sniffs/WP/CronIntervalSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@

namespace WordPressCS\WordPress\Sniffs\WP;

use WordPressCS\WordPress\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Tokens\Collections;
use PHPCSUtils\Utils\Arrays;
use PHPCSUtils\Utils\FunctionDeclarations;
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.
Expand Down Expand Up @@ -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;
}
Expand Down
12 changes: 12 additions & 0 deletions WordPress/Tests/Security/NonceVerificationUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
3 changes: 3 additions & 0 deletions WordPress/Tests/Security/NonceVerificationUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -56,6 +57,8 @@ public function getErrorList() {
202 => 1,
252 => 1,
269 => 1,
306 => 1,
312 => 1,
);
}

Expand Down

0 comments on commit a25d821

Please sign in to comment.