From b54105fe59714e0af40e7758cd2bbc1ffc8b64d0 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 29 Jul 2020 13:35:07 +0200 Subject: [PATCH] RestrictedVariables: don't report on "use" in `isset()` Disregard when the existence of a restricted variable is being checked. This uses the upstream WPCS `Sniff::is_in_isset_or_empty()` method. This means that variables will not be reported as "used" when they are wrapped in a call to: * `isset()` * `empty()` * `array_key_exists()` This also means that the `if ( isset( $_SERVER['REMOTE_ADDR'] ) ) {` test on line 13 will no longer report a warning. As `$_SERVER['REMOTE_ADDR']` was then no longer tested for and `$_SERVER['HTTP_USER_AGENT']` was being tested twice (line 14 and line 28), I've changed the occurrence on line 14 to use `$_SERVER['REMOTE_ADDR']` to make sure both are still tested. Includes additional unit test for the case as reported by the op. Fixes 568 --- WordPress-VIP-Go/ruleset-test.inc | 2 +- .../Sniffs/AbstractVariableRestrictionsSniff.php | 5 +++++ .../Tests/Variables/RestrictedVariablesUnitTest.inc | 8 ++++++-- .../Tests/Variables/RestrictedVariablesUnitTest.php | 1 - 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index d1d1445c..4706542e 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -53,7 +53,7 @@ setcookie( 'cookie[three]', 'cookiethree' ); // Error + Message. $x = sanitize_key( $_COOKIE['bar'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated -- Error + Message. // WordPressVIPMinimum.Variables.RestrictedVariables.cache_constraints___SERVER__HTTP_USER_AGENT__ -if ( ! isset( $_SERVER['HTTP_USER_AGENT'] ) ) { // Error + Message. +if ( isset( $_SERVER['HTTP_USER_AGENT'] ) && $_SERVER['HTTP_USER_AGENT'] === 'some_value' ) { // Error + Message. } // WordPress.WP.AlternativeFunctions.file_system_read_fclose diff --git a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php index bc5e7a46..b4a77874 100644 --- a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php @@ -143,6 +143,11 @@ public function process_token( $stackPtr ) { } } + if ( $this->is_in_isset_or_empty( $stackPtr ) === true ) { + // Checking whether a variable exists is not the same as using it. + return; + } + foreach ( $this->groups_cache as $groupName => $group ) { if ( isset( $this->excluded_groups[ $groupName ] ) ) { diff --git a/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.inc b/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.inc index 8918d631..2126a037 100644 --- a/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.inc @@ -10,8 +10,8 @@ $wp_db->update( $wpdb->usermeta, array( 'meta_value' => 'bar!' ), array( 'user_i $query = "SELECT * FROM $wpdb->posts"; // Ok. -if ( isset( $_SERVER['REMOTE_ADDR'] ) ) { // Warning. - foo( $_SERVER['HTTP_USER_AGENT'] ); // Warning. +if ( isset( $_SERVER['REMOTE_ADDR'] ) ) { // OK. + foo( $_SERVER['REMOTE_ADDR'] ); // Warning. } $x = $_COOKIE['bar']; // Warning. @@ -35,3 +35,7 @@ $query = "SELECT * FROM $wpdb->usermeta"; // Ok, excluded. foo( $_SESSION ); // Error. foo( $_SESSION['bar'] ); // Error. + +if ( isset( $_SESSION ) ) { // OK. + $cache = false; +} diff --git a/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php b/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php index 414349f5..37ef4079 100644 --- a/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php +++ b/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php @@ -43,7 +43,6 @@ public function getErrorList() { */ public function getWarningList() { return [ - 13 => 1, 14 => 1, 17 => 1, 28 => 1,