From 2338970280a2580932fed259c05671192a83fd94 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 26 Jul 2025 17:51:28 +0200 Subject: [PATCH 1/6] Security/EscapingVoidReturnFunctions: extend AbstractFunctionParameterSniff As things were, the determination of whether or not a `T_STRING` is a call to any of the global WP `esc_*()` or `wp_kses*()` functions, was severely flawed. By switching the sniff over to be based on the WordPressCS `AbstractFunctionParameterSniff` class, this flaw is mitigated. Take note of the `$target_functions` using wildcards to match all function calls starting with the prefixes indicates (as it was before). Includes adding a slew of additional tests, some of which (line 8 - 15, line 52, 60, 63) are specific to the flaw being addressed in this commit. Additionally, the tests have been made more comprehensive and varied by: * Testing against false positives for calls to methods or namespaced function calls (= the issue being addressed in this PR). * Ensure function import `use` statements are not flagged. We're not interested in those. * Safeguarding that function calls using PHP 5.6+ argument unpacking are not flagged. * Safeguarding handling of PHP 8.0+ attribute class using the same name as a target function. * Safeguarding that the function is not flagged when used as a PHP 8.1+ first class callable. * Adding tests with more variations: - Variation in the escaping functions being called. - Variation in the printing functions being called. - Non-lowercase function call(s), for both the escaping functions as well as the printing functions. This was previously handled correctly for the printing functions (via WPCS), but not for the escaping functions. - Fully qualified function calls for the escaping functions. - Use PHP 7.3+ trailing comma's in a few function calls. - Multi-line function call. - Comments in unexpected places. --- .../EscapingVoidReturnFunctionsSniff.php | 41 +++++------ .../EscapingVoidReturnFunctionsUnitTest.inc | 68 ++++++++++++++++++- .../EscapingVoidReturnFunctionsUnitTest.php | 18 ++++- 3 files changed, 105 insertions(+), 22 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index 02d2ab6d..fda5e9f5 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -10,8 +10,8 @@ namespace WordPressVIPMinimum\Sniffs\Security; use PHP_CodeSniffer\Util\Tokens; +use WordPressCS\WordPress\AbstractFunctionParameterSniff; use WordPressCS\WordPress\Helpers\PrintingFunctionsTrait; -use WordPressVIPMinimum\Sniffs\Sniff; /** * Flag functions that don't return anything, yet are wrapped in an escaping function call. @@ -20,37 +20,40 @@ * * @uses \WordPressCS\WordPress\Helpers\PrintingFunctionsTrait::$customPrintingFunctions */ -class EscapingVoidReturnFunctionsSniff extends Sniff { +class EscapingVoidReturnFunctionsSniff extends AbstractFunctionParameterSniff { use PrintingFunctionsTrait; /** - * Returns an array of tokens this test wants to listen for. + * The group name for this group of functions. * - * @return array + * @var string */ - public function register() { - return [ - T_STRING, - ]; - } + protected $group_name = 'escaping_void'; + + /** + * Functions this sniff is looking for. + * + * @var array Keys are target functions, value irrelevant. + */ + protected $target_functions = [ + 'esc_*' => true, + 'wp_kses*' => true, + ]; /** - * Process this test when one of its tokens is encountered + * Process the parameters of a matched function. * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * @param array $parameters Array with information about the parameters. * * @return void */ - public function process_token( $stackPtr ) { - - if ( strpos( $this->tokens[ $stackPtr ]['content'], 'esc_' ) !== 0 && strpos( $this->tokens[ $stackPtr ]['content'], 'wp_kses' ) !== 0 ) { - // Not what we are looking for. - return; - } - + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { $next_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); - if ( $this->tokens[ $next_token ]['code'] !== T_OPEN_PARENTHESIS ) { // Not a function call. return; diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc index 9a9de966..1276894d 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc @@ -1,4 +1,68 @@ esc_js( _deprecated_file() ); +$this?->esc_textarea( _e( $something ) ); +MyClass::esc_url( wp_die(), ); +echo WP_KSES; +namespace\wp_kses_post( user_error( $something ) ); + +function esc_js( _E $param ) {} // Class "_E" as type declaration. + +/* + * These should all be okay. + */ +// Incomplete function call, should be ignored by the sniff. +$incorrect_but_ok = esc_html(); +$incorrect_but_ok = wp_kses(); + +// Parameter does not contain a function call. +esc_html_x( $hook_name ); +\wp_kses_data( $obj->_deprecated_function() ); +esc_attr__( CONSTANT_NAME, ); +\ESC_ATTR_X( "do_$something" ); + +esc_attr(...$params); // PHP 5.6 argument unpacking. + +#[Esc_Textarea('text')] // PHP 8.0+ class instantiation via an attribute. Can't contain a nested function call anyway. +function foo() {} + +array_walk($text_strings, \esc_html__(...),); // PHP 8.1 first class callable. + +// Parameter contains a function call but not calling one of the printing functions. +esc_html( __( $something ) ); +Esc_URL_Raw( \get( $url ) ); +\esc_xml( deprecated_argument() ); // Note: missing "_" prefix for printing function. +wp_kses_one_attr ( + /* comment */ + get_attr(), +); + + +/* + * These should all be flagged. + */ +esc_html( _e( $something ) ); +esc_attr__( _deprecated_argument( $a ) ); +ESC_ATTR_E( _Deprecated_Constructor($a), ); +\esc_attr_x( _deprecated_file(), ); +esc_attr( _deprecated_function() ); +esc_HTML__( _deprecated_hook() ); +esc_html_e( _Doing_It_Wrong( $a ) ); +esc_html_X( _e( $foo ), ); +\esc_html( /*comment*/ _ex( $foo ) ); +esc_js( printf( $foo ) ); +Esc_textarea( trigger_error( $foo ) ); +\esc_URL_raw( user_Error( $foo, '' ), ); +esc_url( vprintf( $foo, ) /*comment*/ ); +WP_Kses_Data( WP_DIE( $foo ), ); +wp_kses_one_attr /*comment*/ ( wp_dropdown_pages( $pages ) ); +wp_kses_post( + _deprecated_function( $fn ) +); +wp_kses( _ex() ); diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php index 52734c3f..7b1b6106 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php @@ -23,7 +23,23 @@ class EscapingVoidReturnFunctionsUnitTest extends AbstractSniffUnitTest { */ public function getErrorList() { return [ - 3 => 1, + 50 => 1, + 51 => 1, + 52 => 1, + 53 => 1, + 54 => 1, + 55 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 62 => 1, + 63 => 1, + 64 => 1, + 65 => 1, + 68 => 1, ]; } From b01a801f5e6e16fc774469a526af0111c4868a9b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 26 Jul 2025 18:07:38 +0200 Subject: [PATCH 2/6] Security/EscapingVoidReturnFunctions: document support for `$customPrintingFunctions` As the sniff uses the WPCS `WordPressCS\WordPress\Helpers\PrintingFunctionsTrait`, a `public` (user-settable) `$customPrintingFunctions` property is automatically supported. This commit adds tests documenting this. --- .../Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc | 6 ++++++ .../Tests/Security/EscapingVoidReturnFunctionsUnitTest.php | 2 ++ 2 files changed, 8 insertions(+) diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc index 1276894d..0f68e1a0 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc @@ -66,3 +66,9 @@ wp_kses_post( _deprecated_function( $fn ) ); wp_kses( _ex() ); + +// Adding custom printing functions is supported. +// phpcs:set WordPressVIPMinimum.Security.EscapingVoidReturnFunctions customPrintingFunctions[] to_screen,my_print +esc_attr( to_screen( $var1 ) ); +\wp_kses_post( my_print() ); +// phpcs:set WordPressVIPMinimum.Security.EscapingVoidReturnFunctions customPrintingFunctions[] diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php index 7b1b6106..14636677 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php @@ -40,6 +40,8 @@ public function getErrorList() { 64 => 1, 65 => 1, 68 => 1, + 72 => 1, + 73 => 1, ]; } From 0193bee16b16abdbeddcf7be1c062f77640c463f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 26 Jul 2025 18:21:04 +0200 Subject: [PATCH 3/6] Security/EscapingVoidReturnFunctions: bug fix - false negative with fully qualified printing functions As things were, the sniff would not recognize nested printing functions if the printing function was a fully qualified function call. Fixed now. Safeguarded by updating some of the pre-existing tests. --- .../Sniffs/Security/EscapingVoidReturnFunctionsSniff.php | 5 ++++- .../Security/EscapingVoidReturnFunctionsUnitTest.inc | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index fda5e9f5..7df88e54 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -59,7 +59,10 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p return; } - $next_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $next_token + 1, null, true ); + $ignore = Tokens::$emptyTokens; + $ignore[ T_NS_SEPARATOR ] = T_NS_SEPARATOR; + + $next_token = $this->phpcsFile->findNext( $ignore, $next_token + 1, null, true ); if ( $this->tokens[ $next_token ]['code'] !== T_STRING ) { // Not what we are looking for. diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc index 0f68e1a0..c635f49c 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc @@ -48,22 +48,22 @@ wp_kses_one_attr ( * These should all be flagged. */ esc_html( _e( $something ) ); -esc_attr__( _deprecated_argument( $a ) ); +esc_attr__( \_deprecated_argument( $a ) ); ESC_ATTR_E( _Deprecated_Constructor($a), ); \esc_attr_x( _deprecated_file(), ); esc_attr( _deprecated_function() ); esc_HTML__( _deprecated_hook() ); esc_html_e( _Doing_It_Wrong( $a ) ); -esc_html_X( _e( $foo ), ); +esc_html_X( \_e( $foo ), ); \esc_html( /*comment*/ _ex( $foo ) ); esc_js( printf( $foo ) ); Esc_textarea( trigger_error( $foo ) ); -\esc_URL_raw( user_Error( $foo, '' ), ); +\esc_URL_raw( \user_Error( $foo, '' ), ); esc_url( vprintf( $foo, ) /*comment*/ ); WP_Kses_Data( WP_DIE( $foo ), ); wp_kses_one_attr /*comment*/ ( wp_dropdown_pages( $pages ) ); wp_kses_post( - _deprecated_function( $fn ) + \_deprecated_function( $fn ) ); wp_kses( _ex() ); From 8bf925f33a7382185daeda3a33c5d6ce85af416a Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 26 Jul 2025 18:26:14 +0200 Subject: [PATCH 4/6] Security/EscapingVoidReturnFunctions: bug fix - false negative for `tag_escape()` The `tag_escape()` function is also one of the WP native escaping functions, but was not examined by this sniff. Fixed now. Includes test. Ref: * https://developer.wordpress.org/reference/functions/tag_escape/ --- .../Sniffs/Security/EscapingVoidReturnFunctionsSniff.php | 5 +++-- .../Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc | 3 +++ .../Tests/Security/EscapingVoidReturnFunctionsUnitTest.php | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index 7df88e54..7929251d 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -37,8 +37,9 @@ class EscapingVoidReturnFunctionsSniff extends AbstractFunctionParameterSniff { * @var array Keys are target functions, value irrelevant. */ protected $target_functions = [ - 'esc_*' => true, - 'wp_kses*' => true, + 'esc_*' => true, + 'tag_escape' => true, + 'wp_kses*' => true, ]; /** diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc index c635f49c..4a7f3303 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc @@ -72,3 +72,6 @@ wp_kses( _ex() ); esc_attr( to_screen( $var1 ) ); \wp_kses_post( my_print() ); // phpcs:set WordPressVIPMinimum.Security.EscapingVoidReturnFunctions customPrintingFunctions[] + +tag_escape( $tag ); // OK. +tag_escape( _e() ); // Bad. diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php index 14636677..a1ac0e1f 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php @@ -42,6 +42,7 @@ public function getErrorList() { 68 => 1, 72 => 1, 73 => 1, + 77 => 1, ]; } From ab01e6f5ad6df88b0ef7ef098c57c87a72e5928c Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 26 Jul 2025 18:34:39 +0200 Subject: [PATCH 5/6] Security/EscapingVoidReturnFunctions: bug fix - false positive for printing, not functions As things were, the sniff would presume that if the first non-empty token within the escaping function call was a `T_STRING`, it would be a function call. As the `T_STRING` token is used for quite a few things, this would lead to false positives. Fixed now by making sure there is an open parenthesis after the `T_STRING`. Includes test. Ref: * https://developer.wordpress.org/reference/functions/tag_escape/ --- .../Sniffs/Security/EscapingVoidReturnFunctionsSniff.php | 7 ++++++- .../Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index 7929251d..20de24ad 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -64,12 +64,17 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p $ignore[ T_NS_SEPARATOR ] = T_NS_SEPARATOR; $next_token = $this->phpcsFile->findNext( $ignore, $next_token + 1, null, true ); - if ( $this->tokens[ $next_token ]['code'] !== T_STRING ) { // Not what we are looking for. return; } + $next_after = $this->phpcsFile->findNext(Tokens::$emptyTokens, $next_token + 1, null, true ); + if ( $this->tokens[ $next_after ]['code'] !== T_OPEN_PARENTHESIS ) { + // Not a function call inside the escaping function. + return; + } + if ( $this->is_printing_function( $this->tokens[ $next_token ]['content'] ) ) { $message = 'Attempting to escape `%s()` which is printing its output.'; $data = [ $this->tokens[ $next_token ]['content'] ]; diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc index 4a7f3303..cc9273f6 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc @@ -75,3 +75,7 @@ esc_attr( to_screen( $var1 ) ); tag_escape( $tag ); // OK. tag_escape( _e() ); // Bad. + +// Bug: these are not function calls inside. +esc_attr__( User_Error::CONSTANT_NAME, ); // OK. +esc_js( _doing_it_wrong::class ); // OK, PHP 5.5 ::class resolution. From e17d08baa35c8e49d19807db8c841189b9616e48 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 26 Jul 2025 23:32:55 +0200 Subject: [PATCH 6/6] Security/EscapingVoidReturnFunctions: add support for PHP 8.0+ named parameters To allow support for named parameters, we need to know the position and name of the parameter to examine within a function call. Previously, this sniff used "wildcard" matching for `esc_*` and `wp_kses*` functions. While the parameter position is the same for all of these, the parameter name is not, so we can no longer use wildcard matching if we want the sniff to support PHP 8.0+ function calls using named parameters. To this end: 1. Change the `$target_functions` property to be explicit about the functions the sniff is targetting. Notes: - I've included all WP native `esc_*` functions with the exception of `esc_sql()` which doesn't feel like it belongs in this list. Please let me know if you prefer that `esc_sql()` is still included. - I've included all WP native `wp_kses_*` functions with the exception of `wp_kses_allowed_html()` which is not an escaping function. - Also take note that this also means that custom/user defined `esc_*`/`wp_kses*` functions wrapping printing functions will no longer be flagged. 2. Changed the `$target_functions` property to contain information about the target parameter name and position. 3. Adjusted the logic in the sniff to allow for named parameters using the new PHPCSUtils 1.0.0-alpha4 `PassedParameters::getParameterFromStack()` method. Includes additional unit tests. --- .../EscapingVoidReturnFunctionsSniff.php | 96 ++++++++++++++++--- .../EscapingVoidReturnFunctionsUnitTest.inc | 19 ++++ .../EscapingVoidReturnFunctionsUnitTest.php | 3 + 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index 20de24ad..ddbdef9e 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -10,6 +10,7 @@ namespace WordPressVIPMinimum\Sniffs\Security; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\PassedParameters; use WordPressCS\WordPress\AbstractFunctionParameterSniff; use WordPressCS\WordPress\Helpers\PrintingFunctionsTrait; @@ -34,12 +35,82 @@ class EscapingVoidReturnFunctionsSniff extends AbstractFunctionParameterSniff { /** * Functions this sniff is looking for. * - * @var array Keys are target functions, value irrelevant. + * @var array Keys are the target functions, + * value, the name and position of the target parameter. */ protected $target_functions = [ - 'esc_*' => true, - 'tag_escape' => true, - 'wp_kses*' => true, + 'esc_attr' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_attr__' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_attr_e' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_attr_x' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html__' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html_e' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html_x' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_js' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_textarea' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_url' => [ + 'param_position' => 1, + 'param_name' => 'url', + ], + 'esc_url_raw' => [ + 'param_position' => 1, + 'param_name' => 'url', + ], + 'esc_xml' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'tag_escape' => [ + 'param_position' => 1, + 'param_name' => 'tag_name', + ], + 'wp_kses' => [ + 'param_position' => 1, + 'param_name' => 'content', + ], + 'wp_kses_data' => [ + 'param_position' => 1, + 'param_name' => 'data', + ], + 'wp_kses_one_attr' => [ + 'param_position' => 1, + 'param_name' => 'attr', + ], + 'wp_kses_post' => [ + 'param_position' => 1, + 'param_name' => 'data', + ], ]; /** @@ -54,23 +125,26 @@ class EscapingVoidReturnFunctionsSniff extends AbstractFunctionParameterSniff { * @return void */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { - $next_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); - if ( $this->tokens[ $next_token ]['code'] !== T_OPEN_PARENTHESIS ) { - // Not a function call. + $param_position = $this->target_functions[ $matched_content ]['param_position']; + $param_name = $this->target_functions[ $matched_content ]['param_name']; + + $target_param = PassedParameters::getParameterFromStack( $parameters, $param_position, $param_name ); + if ( $target_param === false ) { + // Missing (required) target parameter. Probably live coding, nothing to examine (yet). Bow out. return; } $ignore = Tokens::$emptyTokens; $ignore[ T_NS_SEPARATOR ] = T_NS_SEPARATOR; - $next_token = $this->phpcsFile->findNext( $ignore, $next_token + 1, null, true ); - if ( $this->tokens[ $next_token ]['code'] !== T_STRING ) { + $next_token = $this->phpcsFile->findNext( $ignore, $target_param['start'], ( $target_param['end'] + 1 ), true ); + if ( $next_token === false || $this->tokens[ $next_token ]['code'] !== T_STRING ) { // Not what we are looking for. return; } - $next_after = $this->phpcsFile->findNext(Tokens::$emptyTokens, $next_token + 1, null, true ); - if ( $this->tokens[ $next_after ]['code'] !== T_OPEN_PARENTHESIS ) { + $next_after = $this->phpcsFile->findNext( Tokens::$emptyTokens, $next_token + 1, ( $target_param['end'] + 1 ), true ); + if ( $next_after === false || $this->tokens[ $next_after ]['code'] !== T_OPEN_PARENTHESIS ) { // Not a function call inside the escaping function. return; } diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc index cc9273f6..3ab7ba59 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc @@ -79,3 +79,22 @@ tag_escape( _e() ); // Bad. // Bug: these are not function calls inside. esc_attr__( User_Error::CONSTANT_NAME, ); // OK. esc_js( _doing_it_wrong::class ); // OK, PHP 5.5 ::class resolution. + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +esc_attr_x( context: get_context(), domain: _e(),); // OK, well, not really, missing required $text param, but that's not the concern of this sniff. +esc_url_raw(protocols: $protocols, url: $url); // OK. +wp_kses_one_attr(element: $element, att: trigger_error( $foo ) ); // OK, well, not really, typo in param name, but that's not the concern of the sniff. +wp_kses( content: \not_deprecated_function( $fn ) ); // OK. +wp_kses( content: /*comment*/ ); // OK, well, not really, invalid function call, but that's not the concern of the sniff. + +esc_html_x(context: $c, text: _doing_it_wrong() ); // Bad. +wp_kses(allowed_html: $allowed_html, content: printf() ); // Bad. +esc_url(protocols: $protocols, url: vprintf()); // Bad. + +// These are no longer flagged as they are not "escaping" functions for the purpose of this sniff. +esc_sql( trigger_error( $foo ) ); // OK. +wp_kses_allowed_html( _deprecated_function() ); // OK. + +// These are no longer flagged as these are not the WP native escaping functions, so we don't know what parameter to check. +esc_something( trigger_error( $foo ) ); // OK. +wp_kses_page( _deprecated_function() ); // OK. diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php index a1ac0e1f..954e2bb2 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php @@ -43,6 +43,9 @@ public function getErrorList() { 72 => 1, 73 => 1, 77 => 1, + 90 => 1, + 91 => 1, + 92 => 1, ]; }