From 402ac50668ca331774415b956a085554aa446ae1 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Mon, 4 Mar 2019 10:04:47 +0000 Subject: [PATCH] EscapingVoidReturnFunctions: Fix docs and logic - Use the parent `$printingFunctions` list of printing functions, instead of just checking for `_e` and `_ex`. - Fix the class description. - Add ruleset tests for both standards. Fixes #387. --- WordPress-VIP-Go/ruleset-test.inc | 16 ++++++++++++++++ WordPress-VIP-Go/ruleset-test.php | 15 +++++++++++++++ .../EscapingVoidReturnFunctionsSniff.php | 6 ++++-- WordPressVIPMinimum/ruleset-test.inc | 16 ++++++++++++++++ WordPressVIPMinimum/ruleset-test.php | 15 +++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index 7c17ff7f..83933e5e 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -322,4 +322,20 @@ update_site_option( $bar, $foo, true ); // Ok. // WordPressVIPMinimum.Functions.RestrictedFunctions.site_option_add_site_option add_site_option( 'foo', $bar ); // Ok. +// WordPressVIPMinimum.Security.EscapingVoidReturnFunctions.Found +esc_js( _deprecated_argument() ); // Error. +esc_js( _deprecated_constructor() ); // Error. +esc_js( _deprecated_file() ); // Error. +esc_js( _deprecated_function() ); // Error. +esc_js( _deprecated_hook() ); // Error. +esc_js( _doing_it_wrong() ); // Error. +esc_html( _e( 'foo', 'bar' ) ); // Error. +esc_html( _ex( 'foo', 'bar' ) ); // Error. +esc_attr( printf( 'foo', [] ) ); // Error. +esc_attr( trigger_error( 'foo' ) ); // Error (+ warning due to trigger_error() call). +esc_attr( user_error( 'foo', '' ) ); // Error. +esc_attr( vprintf( 'foo', [] ) ); // Error. +esc_attr( wp_die( 'foo' ) ); // Error. +esc_attr( wp_dropdown_pages() ); // Error. + ?> diff --git a/WordPress-VIP-Go/ruleset-test.php b/WordPress-VIP-Go/ruleset-test.php index 6ba4a41d..f73f4b9a 100644 --- a/WordPress-VIP-Go/ruleset-test.php +++ b/WordPress-VIP-Go/ruleset-test.php @@ -62,6 +62,20 @@ 268 => 1, 270 => 1, 271 => 1, + 326 => 1, + 327 => 1, + 328 => 1, + 329 => 1, + 330 => 1, + 331 => 1, + 332 => 1, + 333 => 1, + 334 => 1, + 335 => 1, + 336 => 1, + 337 => 1, + 338 => 1, + 339 => 1, ], 'warnings' => [ 84 => 1, @@ -116,6 +130,7 @@ 277 => 1, 281 => 1, 285 => 1, + 335 => 1, ], 'messages' => [ 4 => [ diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index 72e2609d..c1f57d12 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -12,7 +12,9 @@ use PHP_CodeSniffer\Util\Tokens; /** - * Flag suspicious WP_Query and get_posts params. + * Flag functions that don't return anything, yet are wrapped in an escaping function call. + * + * E.g. esc_html( _e( 'foo' ) ); * * @package VIPCS\WordPressVIPMinimum */ @@ -57,7 +59,7 @@ public function process_token( $stackPtr ) { return; } - if ( 0 === strpos( $this->tokens[ $next_token ]['content'], '_e' ) ) { + if ( isset( $this->printingFunctions[ $this->tokens[ $next_token ]['content'] ] ) ) { $message = 'Attempting to escape `%s()` which is printing its output.'; $data = [ $this->tokens[ $next_token ]['content'] ]; $this->phpcsFile->addError( $message, $stackPtr, 'Found', $data ); diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index 7f96a808..59f6d6d2 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -188,4 +188,20 @@ update_site_option( $bar, $foo, true ); // Error. // WordPressVIPMinimum.Functions.RestrictedFunctions.site_option_delete_site_option delete_site_option( $foo ); // Error. +// WordPressVIPMinimum.Security.EscapingVoidReturnFunctions.Found +esc_js( _deprecated_argument() ); // Error. +esc_js( _deprecated_constructor() ); // Error. +esc_js( _deprecated_file() ); // Error. +esc_js( _deprecated_function() ); // Error. +esc_js( _deprecated_hook() ); // Error. +esc_js( _doing_it_wrong() ); // Error. +esc_html( _e( 'foo', 'bar' ) ); // Error. +esc_html( _ex( 'foo', 'bar' ) ); // Error. +esc_attr( printf( 'foo', [] ) ); // Error. +esc_attr( trigger_error( 'foo' ) ); // Error (+ warning due to trigger_error() call). +esc_attr( user_error( 'foo', '' ) ); // Error. +esc_attr( vprintf( 'foo', [] ) ); // Error. +esc_attr( wp_die( 'foo' ) ); // Error. +esc_attr( wp_dropdown_pages() ); // Error. + ?> diff --git a/WordPressVIPMinimum/ruleset-test.php b/WordPressVIPMinimum/ruleset-test.php index 1bdd9265..85912cfe 100644 --- a/WordPressVIPMinimum/ruleset-test.php +++ b/WordPressVIPMinimum/ruleset-test.php @@ -49,6 +49,20 @@ 183 => 1, 186 => 1, 189 => 1, + 192 => 1, + 193 => 1, + 194 => 1, + 195 => 1, + 196 => 1, + 197 => 1, + 198 => 1, + 199 => 1, + 200 => 1, + 201 => 1, + 202 => 1, + 203 => 1, + 204 => 1, + 205 => 1, ], 'warnings' => [ 18 => 1, @@ -75,6 +89,7 @@ 166 => 1, 170 => 1, 177 => 1, + 201 => 1, ], 'messages' => [ 123 => [