Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move is_in_function_call() utility method to dedicated ContextHelper #2229

Merged
merged 1 commit into from
Apr 20, 2023
Merged

Move is_in_function_call() utility method to dedicated ContextHelper #2229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions WordPress/Helpers/ContextHelper.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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