From d43381a2a57f2e30b68355daf99f3ac590d9deb6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 28 Mar 2019 04:41:50 +0100 Subject: [PATCH] Sniff::is_sanitized(): make the method more code style independent Whether people comply with the WP style rules regarding whitespace and such, is irrelevant for determining whether or not a value is sanitized/unslashed. This fixed a false positive where superglobals which were correctly unslashed/sanitized were not being recognized as such. Includes unit test in the `ValidatedSanitizedInput` sniff. --- WordPress/Sniff.php | 27 ++++++++++--------- .../ValidatedSanitizedInputUnitTest.inc | 2 ++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/WordPress/Sniff.php b/WordPress/Sniff.php index 7d5de2c044..caf12632b2 100644 --- a/WordPress/Sniff.php +++ b/WordPress/Sniff.php @@ -1572,46 +1572,47 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) { if ( $require_unslash ) { $this->add_unslash_error( $stackPtr ); } + return false; } // Get the function that it's in. $nested_parenthesis = $this->tokens[ $stackPtr ]['nested_parenthesis']; - $function_closer = end( $nested_parenthesis ); - $function_opener = key( $nested_parenthesis ); - $function = $this->tokens[ ( $function_opener - 1 ) ]; + $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 === $function['code'] ) { + if ( \T_UNSET === $this->tokens[ $functionPtr ]['code'] ) { return true; } - // If this isn't a call to a function, it sure isn't sanitizing function. - if ( \T_STRING !== $function['code'] ) { + // If this isn't a call to a function, it sure isn't a sanitizing function. + if ( \T_STRING !== $this->tokens[ $functionPtr ]['code'] ) { if ( $require_unslash ) { $this->add_unslash_error( $stackPtr ); } + return false; } - $functionName = $function['content']; + $functionName = $this->tokens[ $functionPtr ]['content']; // Check if wp_unslash() is being used. if ( 'wp_unslash' === $functionName ) { $is_unslashed = true; - $function_closer = prev( $nested_parenthesis ); + $function_opener = array_pop( $nested_openers ); // If there is no other function being used, this value is unsanitized. - if ( ! $function_closer ) { + if ( ! isset( $function_opener ) ) { return false; } - $function_opener = key( $nested_parenthesis ); - $functionName = $this->tokens[ ( $function_opener - 1 ) ]['content']; + $functionPtr = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $function_opener - 1 ), null, true, null, true ); + $functionName = $this->tokens[ $functionPtr ]['content']; } else { - $is_unslashed = false; } @@ -1619,7 +1620,7 @@ protected function is_sanitized( $stackPtr, $require_unslash = false ) { if ( 'array_map' === $functionName ) { // Get the first parameter. - $callback = $this->get_function_call_parameter( ( $function_opener - 1 ), 1 ); + $callback = $this->get_function_call_parameter( $functionPtr, 1 ); if ( ! empty( $callback ) ) { /* diff --git a/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.inc b/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.inc index da3be11da1..d9671b4dfb 100644 --- a/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.inc +++ b/WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.inc @@ -209,3 +209,5 @@ if ( $obj->array_key_exists( 'my_field4', $_POST ) ) { if ( ClassName::array_key_exists( 'my_field5', $_POST ) ) { $id = (int) $_POST['my_field5']; // Bad. } + +echo sanitize_text_field (wp_unslash ($_GET['test'])); // OK.