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

Sniff: move "missing unslashing" handling to callback function #2347

Merged
merged 1 commit into from
Aug 17, 2023
Merged

Sniff: move "missing unslashing" handling to callback function #2347

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
41 changes: 17 additions & 24 deletions WordPress/Sniff.php
View file
Open in desktop
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
View file
Open in desktop
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'] )
);
}
}
Loading