Skip to content

Commit

Permalink
Merge pull request #2347 from WordPress/feature/move-unslash-handling…
Browse files Browse the repository at this point in the history
…-to-sniff
  • Loading branch information
GaryJones authored Aug 17, 2023
2 parents 31cef73 + fa06a59 commit e31c0cb
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 25 deletions.
41 changes: 17 additions & 24 deletions WordPress/Sniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,24 @@ protected function is_only_sanitized( $stackPtr ) {
* 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 bool $require_unslash Whether to give an error if no unslashing function
* is used on the variable before sanitization.
* @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, $require_unslash = false ) {
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 ) ) {
Expand All @@ -162,7 +172,7 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) {
// 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 ) {
$this->add_unslash_error( $stackPtr );
call_user_func( $unslash_callback, $this->phpcsFile, $stackPtr );
}

return false;
Expand All @@ -189,7 +199,7 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) {
// 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 ) {
$this->add_unslash_error( $stackPtr );
call_user_func( $unslash_callback, $this->phpcsFile, $stackPtr );
}

return false;
Expand Down Expand Up @@ -246,7 +256,7 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) {

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

// Check if this is a sanitizing function.
Expand All @@ -256,21 +266,4 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) {

return false;
}

/**
* Add an error for missing use of unslashing.
*
* @since 0.5.0
*
* @param int $stackPtr The index of the token in the stack.
*/
public function add_unslash_error( $stackPtr ) {

$this->phpcsFile->addError(
'%s data not unslashed before sanitization. Use wp_unslash() or similar',
$stackPtr,
'MissingUnslash',
array( $this->tokens[ $stackPtr ]['content'] )
);
}
}
27 changes: 26 additions & 1 deletion WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace WordPressCS\WordPress\Sniffs\Security;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\Helpers\ContextHelper;
Expand Down Expand Up @@ -160,7 +161,7 @@ function ( $embed ) {
}

// Now look for sanitizing functions.
if ( ! $this->is_sanitized( $stackPtr, true ) ) {
if ( ! $this->is_sanitized( $stackPtr, array( $this, 'add_unslash_error' ) ) ) {
$this->phpcsFile->addError(
'Detected usage of a non-sanitized input variable: %s',
$stackPtr,
Expand All @@ -169,4 +170,28 @@ function ( $embed ) {
);
}
}

/**
* Add an error for missing use of unslashing.
*
* @since 0.5.0
* @since 3.0.0 - Moved from the `Sniff` class to this class.
* - The `$phpcsFile` parameter was added.
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The index of the token in the stack
* which is missing unslashing.
*
* @return void
*/
public function add_unslash_error( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

$phpcsFile->addError(
'%s data not unslashed before sanitization. Use wp_unslash() or similar',
$stackPtr,
'MissingUnslash',
array( $tokens[ $stackPtr ]['content'] )
);
}
}

0 comments on commit e31c0cb

Please sign in to comment.