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

Move sanitization methods to SanitizationHelperTrait + minor improvements #2356

Merged
merged 8 commits into from
Aug 18, 2023
Merged

Move sanitization methods to SanitizationHelperTrait + minor improvements #2356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
21d8cae
Rename `SanitizingFunctionsTrait` to `SanitizationHelperTrait`
jrfnl Aug 18, 2023
50e1fe9
Move sanitization methods to `SanitizationHelperTrait`
jrfnl Jul 6, 2023
e72ca2c
SanitizationHelperTrait::is_sanitized(): add some defensive coding
jrfnl Aug 18, 2023
b217ebc
SanitizationHelperTrait::is_sanitized(): implement PHPCSUtils
jrfnl Jul 6, 2023
0834f59
SanitizationHelperTrait::is_sanitized(): efficiency tweak - move unse…
jrfnl Aug 18, 2023
1bf115b
SanitizationHelperTrait::is_sanitized(): efficiency fix/improve perfo…
jrfnl Aug 18, 2023
6440bed
SanitizationHelperTrait::is_sanitized(): minor code tweaks
jrfnl Aug 18, 2023
2d019df
SanitizationHelperTrait::is_sanitized(): minor doc tweaks
jrfnl Aug 18, 2023
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
180 changes: 179 additions & 1 deletion ...ress/Helpers/SanitizingFunctionsTrait.php → ...Press/Helpers/SanitizationHelperTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@

namespace WordPressCS\WordPress\Helpers;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\Context;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\Helpers\ArrayWalkingFunctionsHelper;
use WordPressCS\WordPress\Helpers\ContextHelper;
use WordPressCS\WordPress\Helpers\RulesetPropertyHelper;
use WordPressCS\WordPress\Helpers\UnslashingFunctionsHelper;

/**
* Helper functions and function lists for checking whether a sanitizing function is being used.
Expand All @@ -19,11 +26,20 @@
* - `customSanitizingFunctions`.
* - `customUnslashingSanitizingFunctions`
*
* ---------------------------------------------------------------------------------------------
* This trait is only intended for internal use by WordPressCS and is not part of the public API.
* This also means that it has no promise of backward compatibility. Use at your own risk.
* ---------------------------------------------------------------------------------------------
*
* @internal
*
* @since 3.0.0 The properties in this trait were previously contained partially in the
* `WordPressCS\WordPress\Sniff` class and partially in the `NonceVerificationSniff`
* and the `ValidatedSanitizedInputSniff` classes and have been moved here.
* The pre-existing methods in this trait were previously contained in the
* `WordPressCS\WordPress\Sniff` class and have been moved here.
*/
trait SanitizingFunctionsTrait {
trait SanitizationHelperTrait {

/**
* Custom list of functions that sanitize the values passed to them.
Expand Down Expand Up @@ -236,4 +252,166 @@ public function is_sanitizing_function( $functionName ) {
public function is_sanitizing_and_unslashing_function( $functionName ) {
return isset( $this->get_sanitizing_and_unslashing_functions()[ strtolower( $functionName ) ] );
}

/**
* Check if something is only being sanitized.
*
* @since 0.5.0
* @since 3.0.0 - Moved from the Sniff class to this class.
* - The method visibility was changed from `protected` to `public`.
* - The `$phpcsFile` parameter was added.
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
dingo-d marked this conversation as resolved.
Show resolved Hide resolved
* @param int $stackPtr The index of the token in the stack.
*
* @return bool Whether the token is only within a sanitization.
*/
public function is_only_sanitized( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

// If it isn't being sanitized at all.
if ( ! $this->is_sanitized( $phpcsFile, $stackPtr ) ) {
return false;
}

// If the token isn't in parentheses, we know the value must have only been casted, because
// is_sanitized() would have returned `false` otherwise.
if ( ! isset( $tokens[ $stackPtr ]['nested_parenthesis'] ) ) {
return true;
}

// At this point we're expecting the value to have not been casted. If it
// was, it wasn't *only* casted, because it's also in a function.
if ( ContextHelper::is_safe_casted( $phpcsFile, $stackPtr ) ) {
return false;
}

// The only parentheses should belong to the sanitizing function. If there's
// more than one set, this isn't *only* sanitization.
return ( \count( $tokens[ $stackPtr ]['nested_parenthesis'] ) === 1 );
}

/**
* Check if something is being sanitized.
*
* @since 0.5.0
* @since 3.0.0 - Moved from the Sniff class to this class.
* - The method visibility was changed from `protected` to `public`.
* - The `$phpcsFile` parameter was added.
* - The $require_unslash parameter has been changed from
* a boolean toggle to a ?callable $unslash_callback parameter to
* allow a sniff calling this method to handle their "unslashing"
* related messaging itself.
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The index of the token in the stack.
* @param callable|null $unslash_callback Optional. When passed, this method will check if
* an unslashing function is used on the variable before
* sanitization and if not, the callback will be called
* to handle the missing unslashing.
* The callback will receive the $phpcsFile object and
* the $stackPtr.
* When not passed or `null`, this method will **not**
* check for unslashing issues.
* Defaults to `null` (skip unslashing checks).
*
* @return bool Whether the token is being sanitized.
*/
public function is_sanitized( File $phpcsFile, $stackPtr, $unslash_callback = null ) {
$tokens = $phpcsFile->getTokens();
$require_unslash = is_callable( $unslash_callback );

if ( isset( $tokens[ $stackPtr ] ) === false ) {
return false;
}

// If the variable is just being unset, the value isn't used at all, so it's safe.
if ( Context::inUnset( $phpcsFile, $stackPtr ) ) {
return true;
}

// First we check if it is being casted to a safe value.
if ( ContextHelper::is_safe_casted( $phpcsFile, $stackPtr ) ) {
return true;
}

// If this isn't within a function call, we know already that it's not safe.
if ( ! isset( $tokens[ $stackPtr ]['nested_parenthesis'] ) ) {
if ( $require_unslash ) {
call_user_func( $unslash_callback, $phpcsFile, $stackPtr );
}

return false;
}

$sanitizing_functions = $this->get_sanitizing_functions();
$sanitizing_functions += $this->get_sanitizing_and_unslashing_functions();
$sanitizing_functions += ArrayWalkingFunctionsHelper::get_functions();
$valid_functions = $sanitizing_functions + UnslashingFunctionsHelper::get_functions();

// Get the function that it's in.
$functionPtr = ContextHelper::is_in_function_call( $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 ) {
if ( true === $require_unslash ) {
call_user_func( $unslash_callback, $phpcsFile, $stackPtr );
}

return false;
}

$functionName = $tokens[ $functionPtr ]['content'];

// Check if an unslashing function is being used.
$is_unslashed = false;
if ( UnslashingFunctionsHelper::is_unslashing_function( $functionName ) ) {
$is_unslashed = true;

// Check whether this function call is wrapped within a sanitizing function.
$higherFunctionPtr = ContextHelper::is_in_function_call( $phpcsFile, $functionPtr, $sanitizing_functions );

// If there is no other valid function being used, this value is unsanitized.
if ( false === $higherFunctionPtr ) {
return false;
}

$functionPtr = $higherFunctionPtr;
$functionName = $tokens[ $functionPtr ]['content'];
}

// Arrays might be sanitized via an array walking function using a callback.
if ( ArrayWalkingFunctionsHelper::is_array_walking_function( $functionName ) ) {
// Get the callback parameter.
$callback = ArrayWalkingFunctionsHelper::get_callback_parameter( $phpcsFile, $functionPtr );

if ( ! empty( $callback ) ) {
/*
* If this is a function callback (not a method callback array) and we're able
* to resolve the function name, do so.
*/
$first_non_empty = $phpcsFile->findNext(
Tokens::$emptyTokens,
$callback['start'],
( $callback['end'] + 1 ),
true
);

if ( false !== $first_non_empty && \T_CONSTANT_ENCAPSED_STRING === $tokens[ $first_non_empty ]['code'] ) {
$functionName = TextStrings::stripQuotes( $tokens[ $first_non_empty ]['content'] );
}
}
}

// If slashing is required, give an error.
if ( false === $is_unslashed
&& true === $require_unslash
&& ! $this->is_sanitizing_and_unslashing_function( $functionName )
) {
call_user_func( $unslash_callback, $phpcsFile, $stackPtr );
}

// Check if this is a sanitizing function.
return ( $this->is_sanitizing_function( $functionName ) || $this->is_sanitizing_and_unslashing_function( $functionName ) );
}
}
169 changes: 1 addition & 168 deletions WordPress/Sniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,8 @@

namespace WordPressCS\WordPress;

use PHP_CodeSniffer\Sniffs\Sniff as PHPCS_Sniff;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\Helpers\ArrayWalkingFunctionsHelper;
use WordPressCS\WordPress\Helpers\ContextHelper;
use WordPressCS\WordPress\Helpers\SanitizingFunctionsTrait;
use WordPressCS\WordPress\Helpers\UnslashingFunctionsHelper;
use PHP_CodeSniffer\Sniffs\Sniff as PHPCS_Sniff;

/**
* Represents a PHP_CodeSniffer sniff for sniffing WordPress coding standards.
Expand All @@ -27,8 +21,6 @@
*/
abstract class Sniff implements PHPCS_Sniff {

use SanitizingFunctionsTrait;

/**
* A list of superglobals that incorporate user input.
*
Expand Down Expand Up @@ -107,163 +99,4 @@ protected function init( File $phpcsFile ) {
$this->phpcsFile = $phpcsFile;
$this->tokens = $phpcsFile->getTokens();
}

/**
* Check if something is only being sanitized.
*
* @since 0.5.0
*
* @param int $stackPtr The index of the token in the stack.
*
* @return bool Whether the token is only within a sanitization.
*/
protected function is_only_sanitized( $stackPtr ) {

// If it isn't being sanitized at all.
if ( ! $this->is_sanitized( $stackPtr ) ) {
return false;
}

// If this isn't set, we know the value must have only been casted, because
// is_sanitized() would have returned false otherwise.
if ( ! isset( $this->tokens[ $stackPtr ]['nested_parenthesis'] ) ) {
return true;
}

// At this point we're expecting the value to have not been casted. If it
// was, it wasn't *only* casted, because it's also in a function.
if ( ContextHelper::is_safe_casted( $this->phpcsFile, $stackPtr ) ) {
return false;
}

// The only parentheses should belong to the sanitizing function. If there's
// more than one set, this isn't *only* sanitization.
return ( \count( $this->tokens[ $stackPtr ]['nested_parenthesis'] ) === 1 );
}

/**
* Check if something is being sanitized.
*
* @since 0.5.0
* @since 3.0.0 The second ($require_unslash) parameter has been changed from
* a boolean toggle to a ?callable $unslash_callback parameter to
* allow a sniff calling this method to handle their "unslashing"
* related messaging itself.
*
* @param int $stackPtr The index of the token in the stack.
* @param callable|null $unslash_callback Optional. When passed, this method will check if an unslashing
* function is used on the variable before sanitization and if not,
* the callback will be called to handle the missing unslashing.
* The callback will receive the $phpcsFile object and the $stackPtr.
* When not passed or `null`, this method will **not** check
* for unslashing issues.
* Defaults to `null` (skip unslashing checks).
*
* @return bool Whether the token being sanitized.
*/
protected function is_sanitized( $stackPtr, $unslash_callback = null ) {
$require_unslash = is_callable( $unslash_callback );

// First we check if it is being casted to a safe value.
if ( ContextHelper::is_safe_casted( $this->phpcsFile, $stackPtr ) ) {
return true;
}

// If this isn't within a function call, we know already that it's not safe.
if ( ! isset( $this->tokens[ $stackPtr ]['nested_parenthesis'] ) ) {
if ( $require_unslash ) {
call_user_func( $unslash_callback, $this->phpcsFile, $stackPtr );
}

return false;
}

// Get the function that it's in.
$nested_parenthesis = $this->tokens[ $stackPtr ]['nested_parenthesis'];
$nested_openers = array_keys( $nested_parenthesis );
$function_opener = array_pop( $nested_openers );
$functionPtr = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $function_opener - 1 ), null, true, null, true );

// If it is just being unset, the value isn't used at all, so it's safe.
if ( \T_UNSET === $this->tokens[ $functionPtr ]['code'] ) {
return true;
}

$valid_functions = $this->get_sanitizing_functions();
$valid_functions += $this->get_sanitizing_and_unslashing_functions();
$valid_functions += UnslashingFunctionsHelper::get_functions();
$valid_functions += ArrayWalkingFunctionsHelper::get_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 ) {
if ( true === $require_unslash ) {
call_user_func( $unslash_callback, $this->phpcsFile, $stackPtr );
}

return false;
}

$functionName = $this->tokens[ $functionPtr ]['content'];

// Check if an unslashing function is being used.
if ( UnslashingFunctionsHelper::is_unslashing_function( $functionName ) ) {

$is_unslashed = true;

// Remove the unslashing functions.
$valid_functions = array_diff_key( $valid_functions, UnslashingFunctionsHelper::get_functions() );

// Check is any of the remaining (sanitizing) functions is used.
$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 ) {
return false;
}

$functionPtr = $higherFunctionPtr;
$functionName = $this->tokens[ $functionPtr ]['content'];

} else {
$is_unslashed = false;
}

// Arrays might be sanitized via an array walking function using a callback.
if ( ArrayWalkingFunctionsHelper::is_array_walking_function( $functionName ) ) {

// Get the callback parameter.
$callback = ArrayWalkingFunctionsHelper::get_callback_parameter( $this->phpcsFile, $functionPtr );

if ( ! empty( $callback ) ) {
/*
* If this is a function callback (not a method callback array) and we're able
* to resolve the function name, do so.
*/
$first_non_empty = $this->phpcsFile->findNext(
Tokens::$emptyTokens,
$callback['start'],
( $callback['end'] + 1 ),
true
);

if ( false !== $first_non_empty && \T_CONSTANT_ENCAPSED_STRING === $this->tokens[ $first_non_empty ]['code'] ) {
$functionName = TextStrings::stripQuotes( $this->tokens[ $first_non_empty ]['content'] );
}
}
}

// If slashing is required, give an error.
if ( ! $is_unslashed && $require_unslash && ! $this->is_sanitizing_and_unslashing_function( $functionName ) ) {
call_user_func( $unslash_callback, $this->phpcsFile, $stackPtr );
}

// Check if this is a sanitizing function.
if ( $this->is_sanitizing_function( $functionName ) || $this->is_sanitizing_and_unslashing_function( $functionName ) ) {
return true;
}

return false;
}
}
Loading