diff --git a/WordPress/Helpers/SanitizingFunctionsTrait.php b/WordPress/Helpers/SanitizationHelperTrait.php similarity index 50% rename from WordPress/Helpers/SanitizingFunctionsTrait.php rename to WordPress/Helpers/SanitizationHelperTrait.php index 368a6fc5e1..9760eca67c 100644 --- a/WordPress/Helpers/SanitizingFunctionsTrait.php +++ b/WordPress/Helpers/SanitizationHelperTrait.php @@ -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. @@ -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. @@ -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. + * @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 ) ); + } } diff --git a/WordPress/Sniff.php b/WordPress/Sniff.php index 7bd96029c7..8b3ad0507d 100644 --- a/WordPress/Sniff.php +++ b/WordPress/Sniff.php @@ -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. @@ -27,8 +21,6 @@ */ abstract class Sniff implements PHPCS_Sniff { - use SanitizingFunctionsTrait; - /** * A list of superglobals that incorporate user input. * @@ -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; - } } diff --git a/WordPress/Sniffs/Security/NonceVerificationSniff.php b/WordPress/Sniffs/Security/NonceVerificationSniff.php index 0909fc257b..0241147327 100644 --- a/WordPress/Sniffs/Security/NonceVerificationSniff.php +++ b/WordPress/Sniffs/Security/NonceVerificationSniff.php @@ -17,6 +17,7 @@ use PHPCSUtils\Utils\Scopes; use WordPressCS\WordPress\Helpers\ContextHelper; use WordPressCS\WordPress\Helpers\RulesetPropertyHelper; +use WordPressCS\WordPress\Helpers\SanitizationHelperTrait; use WordPressCS\WordPress\Helpers\UnslashingFunctionsHelper; use WordPressCS\WordPress\Helpers\VariableHelper; use WordPressCS\WordPress\Sniff; @@ -31,11 +32,13 @@ * @since 1.0.0 This sniff has been moved from the `CSRF` category to the `Security` category. * @since 3.0.0 This sniff has received significant updates to its logic and structure. * - * @uses \WordPressCS\WordPress\Helpers\SanitizingFunctionsTrait::$customSanitizingFunctions - * @uses \WordPressCS\WordPress\Helpers\SanitizingFunctionsTrait::$customUnslashingSanitizingFunctions + * @uses \WordPressCS\WordPress\Helpers\SanitizationHelperTrait::$customSanitizingFunctions + * @uses \WordPressCS\WordPress\Helpers\SanitizationHelperTrait::$customUnslashingSanitizingFunctions */ class NonceVerificationSniff extends Sniff { + use SanitizationHelperTrait; + /** * Superglobals to notify about when not accompanied by an nonce check. * @@ -234,7 +237,7 @@ protected function needs_nonce_check( $stackPtr, array $cache_keys ) { || VariableHelper::is_assignment( $this->phpcsFile, $stackPtr, true ) || ContextHelper::is_in_array_comparison( $this->phpcsFile, $stackPtr ) || ContextHelper::is_in_function_call( $this->phpcsFile, $stackPtr, UnslashingFunctionsHelper::get_functions() ) !== false - || $this->is_only_sanitized( $stackPtr ) + || $this->is_only_sanitized( $this->phpcsFile, $stackPtr ) ) { $needs_nonce = 'after'; } diff --git a/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php b/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php index 1bcb5060c2..cbbcab1059 100644 --- a/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php +++ b/WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php @@ -13,6 +13,7 @@ use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\Helpers\ContextHelper; +use WordPressCS\WordPress\Helpers\SanitizationHelperTrait; use WordPressCS\WordPress\Helpers\ValidationHelper; use WordPressCS\WordPress\Helpers\VariableHelper; use WordPressCS\WordPress\Sniff; @@ -28,11 +29,13 @@ * @since 0.13.0 Class name changed: this class is now namespaced. * @since 1.0.0 This sniff has been moved from the `VIP` category to the `Security` category. * - * @uses \WordPressCS\WordPress\Helpers\SanitizingFunctionsTrait::$customSanitizingFunctions - * @uses \WordPressCS\WordPress\Helpers\SanitizingFunctionsTrait::$customUnslashingSanitizingFunctions + * @uses \WordPressCS\WordPress\Helpers\SanitizationHelperTrait::$customSanitizingFunctions + * @uses \WordPressCS\WordPress\Helpers\SanitizationHelperTrait::$customUnslashingSanitizingFunctions */ class ValidatedSanitizedInputSniff extends Sniff { + use SanitizationHelperTrait; + /** * Check for validation functions for a variable within its own parenthesis only. * @@ -161,7 +164,7 @@ function ( $embed ) { } // Now look for sanitizing functions. - if ( ! $this->is_sanitized( $stackPtr, array( $this, 'add_unslash_error' ) ) ) { + if ( ! $this->is_sanitized( $this->phpcsFile, $stackPtr, array( $this, 'add_unslash_error' ) ) ) { $this->phpcsFile->addError( 'Detected usage of a non-sanitized input variable: %s', $stackPtr, diff --git a/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.php b/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.php index 87e8776789..dd320c8a3f 100644 --- a/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.php +++ b/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.php @@ -19,7 +19,7 @@ * @since 1.0.0 This sniff has been moved from the `VIP` category to the `Security` category. * * @covers \WordPressCS\WordPress\Helpers\ArrayWalkingFunctionsHelper - * @covers \WordPressCS\WordPress\Helpers\SanitizingFunctionsTrait + * @covers \WordPressCS\WordPress\Helpers\SanitizationHelperTrait * @covers \WordPressCS\WordPress\Helpers\UnslashingFunctionsHelper * @covers \WordPressCS\WordPress\Helpers\ValidationHelper * @covers \WordPressCS\WordPress\Helpers\VariableHelper