Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix - Lowercase custom sniff properties #2391

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions WordPress/Helpers/EscapingFunctionsTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ final public function is_escaping_function( $functionName ) {
) {
$this->allEscapingFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customEscapingFunctions,
$this->escapingFunctions
$this->escapingFunctions,
true,
true
);

$this->addedCustomEscapingFunctions['escape'] = $this->customEscapingFunctions;
Expand All @@ -244,7 +246,9 @@ final public function is_auto_escaped_function( $functionName ) {
) {
$this->allAutoEscapedFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customAutoEscapedFunctions,
$this->autoEscapedFunctions
$this->autoEscapedFunctions,
true,
true
);

$this->addedCustomEscapingFunctions['autoescape'] = $this->customAutoEscapedFunctions;
Expand Down
10 changes: 3 additions & 7 deletions WordPress/Helpers/IsUnitTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,11 @@ final protected function get_all_test_classes() {
}
}

/*
* Lowercase all names, both custom as well as "known", as PHP treats namespaced names case-insensitively.
*/
$custom_test_classes = array_map( 'strtolower', $custom_test_classes );
$known_test_classes = array_change_key_case( $this->known_test_classes, \CASE_LOWER );

$this->all_test_classes = RulesetPropertyHelper::merge_custom_array(
$custom_test_classes,
$known_test_classes
$this->known_test_classes,
true,
true
);

// Store the original value so the comparison can succeed.
Expand Down
4 changes: 3 additions & 1 deletion WordPress/Helpers/PrintingFunctionsTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ final public function get_printing_functions() {
) {
$this->allPrintingFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customPrintingFunctions,
$this->printingFunctions
$this->printingFunctions,
true,
true
);

$this->addedCustomPrintingFunctions = $this->customPrintingFunctions;
Expand Down
26 changes: 20 additions & 6 deletions WordPress/Helpers/RulesetPropertyHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,25 @@ final class RulesetPropertyHelper {
* @since 2.0.0 No longer supports custom array properties which were incorrectly
* passed as a string.
* @since 3.0.0 Moved from the Sniff class to this class.
* @since 3.0.2 Added a new parameter to lowercase array keys and values.
dingo-d marked this conversation as resolved.
Show resolved Hide resolved
*
* @param array $custom Custom list as provided via a ruleset.
* @param array $base Optional. Base list. Defaults to an empty array.
* Expects `value => true` format when `$flip` is true.
* @param bool $flip Optional. Whether or not to flip the custom list.
* Defaults to true.
* @param array $custom Custom list as provided via a ruleset.
* @param array $base Optional. Base list. Defaults to an empty array.
* Expects `value => true` format when `$flip` is true.
* @param bool $flip Optional. Whether or not to flip the custom list.
* @param bool $lowercasekeys Optional. Whether to lowercase keys and values in the resulting array.
dingo-d marked this conversation as resolved.
Show resolved Hide resolved
* Defaults to false.
GaryJones marked this conversation as resolved.
Show resolved Hide resolved
* @return array
*/
public static function merge_custom_array( $custom, array $base = array(), $flip = true ) {
public static function merge_custom_array( $custom, array $base = array(), $flip = true, $lowercasekeys = false ) {
if ( empty( $base ) && empty( $custom ) ) {
return array();
}

if ( $lowercasekeys ) {
$base = array_change_key_case( $base );
}

if ( true === $flip ) {
$base = array_filter( $base );
}
Expand All @@ -64,6 +74,10 @@ public static function merge_custom_array( $custom, array $base = array(), $flip
$custom = array_fill_keys( $custom, false );
}

if ( $lowercasekeys ) {
$custom = array_change_key_case( $custom );
}

if ( empty( $base ) ) {
return $custom;
}
Expand Down
8 changes: 6 additions & 2 deletions WordPress/Helpers/SanitizationHelperTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ final public function get_sanitizing_functions() {
) {
$this->allSanitizingFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customSanitizingFunctions,
$this->sanitizingFunctions
$this->sanitizingFunctions,
true,
true
);

$this->addedCustomSanitizingFunctions['sanitize'] = $this->customSanitizingFunctions;
Expand All @@ -218,7 +220,9 @@ final public function get_sanitizing_and_unslashing_functions() {
) {
$this->allUnslashingSanitizingFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customUnslashingSanitizingFunctions,
$this->unslashingSanitizingFunctions
$this->unslashingSanitizingFunctions,
true,
true
);

$this->addedCustomSanitizingFunctions['unslashsanitize'] = $this->customUnslashingSanitizingFunctions;
Expand Down
19 changes: 13 additions & 6 deletions WordPress/Sniffs/DB/DirectDatabaseQuerySniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,18 +229,19 @@ public function process_token( $stackPtr ) {

for ( $i = ( $scopeStart + 1 ); $i < $scopeEnd; $i++ ) {
if ( \T_STRING === $this->tokens[ $i ]['code'] ) {
$content = strtolower( $this->tokens[ $i ]['content'] );

if ( isset( $this->cacheDeleteFunctions[ $this->tokens[ $i ]['content'] ] ) ) {
if ( isset( $this->cacheDeleteFunctions[ $content ] ) ) {

if ( \in_array( $method, array( 'query', 'update', 'replace', 'delete' ), true ) ) {
$cached = true;
break;
}
} elseif ( isset( $this->cacheGetFunctions[ $this->tokens[ $i ]['content'] ] ) ) {
} elseif ( isset( $this->cacheGetFunctions[ $content ] ) ) {

$wp_cache_get = true;

} elseif ( isset( $this->cacheSetFunctions[ $this->tokens[ $i ]['content'] ] ) ) {
} elseif ( isset( $this->cacheSetFunctions[ $content ] ) ) {

if ( $wp_cache_get ) {
$cached = true;
Expand Down Expand Up @@ -274,7 +275,9 @@ protected function mergeFunctionLists() {
if ( $this->customCacheGetFunctions !== $this->addedCustomFunctions['cacheget'] ) {
$this->cacheGetFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customCacheGetFunctions,
$this->cacheGetFunctions
$this->cacheGetFunctions,
true,
true
);

$this->addedCustomFunctions['cacheget'] = $this->customCacheGetFunctions;
Expand All @@ -283,7 +286,9 @@ protected function mergeFunctionLists() {
if ( $this->customCacheSetFunctions !== $this->addedCustomFunctions['cacheset'] ) {
$this->cacheSetFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customCacheSetFunctions,
$this->cacheSetFunctions
$this->cacheSetFunctions,
true,
true
);

$this->addedCustomFunctions['cacheset'] = $this->customCacheSetFunctions;
Expand All @@ -292,7 +297,9 @@ protected function mergeFunctionLists() {
if ( $this->customCacheDeleteFunctions !== $this->addedCustomFunctions['cachedelete'] ) {
$this->cacheDeleteFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customCacheDeleteFunctions,
$this->cacheDeleteFunctions
$this->cacheDeleteFunctions,
true,
true
);

$this->addedCustomFunctions['cachedelete'] = $this->customCacheDeleteFunctions;
Expand Down
6 changes: 4 additions & 2 deletions WordPress/Sniffs/Security/NonceVerificationSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ private function has_nonce_check( $stackPtr, array $cache_keys, $allow_nonce_aft
}

// If this is one of the nonce verification functions, we can bail out.
if ( isset( $this->nonceVerificationFunctions[ $this->tokens[ $i ]['content'] ] ) ) {
if ( isset( $this->nonceVerificationFunctions[ strtolower( $this->tokens[ $i ]['content'] ) ] ) ) {
/*
* Now, make sure it is a call to a global function.
*/
Expand Down Expand Up @@ -413,7 +413,9 @@ protected function mergeFunctionLists() {
if ( $this->customNonceVerificationFunctions !== $this->addedCustomNonceFunctions ) {
$this->nonceVerificationFunctions = RulesetPropertyHelper::merge_custom_array(
$this->customNonceVerificationFunctions,
$this->nonceVerificationFunctions
$this->nonceVerificationFunctions,
true,
true
);

$this->addedCustomNonceFunctions = $this->customNonceVerificationFunctions;
Expand Down
60 changes: 60 additions & 0 deletions WordPress/Tests/DB/DirectDatabaseQueryUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,66 @@ function method_names_are_caseinsensitive() {
$autoload = $wpdb->Get_Var( $wpdb->Prepare( "SELECT autoload FROM $wpdb->options WHERE option_name = %s", $option_name ) ); // Warning x 2.
}

/*
* Test using custom properties, setting & unsetting (resetting).
*/
// phpcs:set WordPress.DB.DirectDatabaseQuery customCacheGetFunctions[] my_cacHeget
// phpcs:set WordPress.DB.DirectDatabaseQuery customCacheSetFunctions[] my_cachEset,my_other_cacheSET
// phpcs:set WordPress.DB.DirectDatabaseQuery customCacheDeleteFunctions[] MY_cachedeL
function cache_customA() {
global $wpdb;
$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // Warning direct DB call.
my_cacheset( 'key', 'group' );
}
}

function cache_customB() {
global $wpdb;
$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // Warning direct DB call.
my_other_cacheset( 'key', 'group' );
}
}

function cache_customC() {
global $wpdb;
$wpdb->query( 'SELECT X FROM Y' ); // Warning direct DB call.
my_cachedel( 'key', 'group' );
}

// phpcs:set WordPress.DB.DirectDatabaseQuery customCacheSetFunctions[] my_cacheSet
// phpcs:set WordPress.DB.DirectDatabaseQuery customCacheDeleteFunctions[]

function cache_customD() {
global $wpdb;
$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // Warning direct DB call.
my_cacheset( 'key', 'group' );
}
}

function cache_customE() {
global $wpdb;
$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // Warning x 2.
my_other_cacheset( 'key', 'group' );
}
}

function cache_customF() {
global $wpdb;
$wpdb->query( 'SELECT X FROM Y' ); // Warning x 2.
my_cachedel( 'key', 'group' );
}

// phpcs:set WordPress.DB.DirectDatabaseQuery customCacheGetFunctions[]
// phpcs:set WordPress.DB.DirectDatabaseQuery customCacheSetFunctions[]

// Live coding/parse error test.
// This must be the last test in the file.
$wpdb->get_col( '
6 changes: 6 additions & 0 deletions WordPress/Tests/DB/DirectDatabaseQueryUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ public function getWarningList() {
300 => 1,
306 => 2,
333 => 2,
346 => 1,
355 => 1,
362 => 1,
373 => 1,
382 => 2,
389 => 2,
);
}
}
2 changes: 1 addition & 1 deletion WordPress/Tests/PHP/NoSilencedErrorsUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ $files1 = @ & scandir($dir); // Bad.
/*
* The custom allowed functions list will be respected even when `usePHPFunctionsList` is set to false.
*/
// phpcs:set WordPress.PHP.NoSilencedErrors customAllowedFunctionsList[] fgetcsv,hex2bin
// phpcs:set WordPress.PHP.NoSilencedErrors customAllowedFunctionsList[] fgetCsv,hEx2bin
while ( ( $csvdata = @fgetcsv( $handle, 2000, $separator ) ) !== false ) {}
echo @some_userland_function( $param ); // Bad.
$decoded = @hex2bin( $data );
Expand Down
6 changes: 6 additions & 0 deletions WordPress/Tests/Security/EscapeOutputUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -655,3 +655,9 @@ echo '<input type="search" value="' . get_search_query( false ) . '">'; // Bad.
echo '<input type="search" value="' . get_search_query( 0 ) . '">'; // Bad.
echo '<input type="search" value="' . get_search_query( escape: false ) . '">'; // OK, well not really, typo in param name, but that's not our concern.
echo '<input type="search" value="' . get_search_query( escaped: false ) . '">'; // Bad.

// phpcs:set WordPress.Security.EscapeOutput customPrintingFunctions[] TO_SCREEN,my_print
to_screen( $var1, esc_attr( $var2 ) ); // Bad x 1.
my_print( $var1, $var2 ); // Bad x 2.

// phpcs:set WordPress.Security.EscapeOutput customEscapingFunctions[]
2 changes: 2 additions & 0 deletions WordPress/Tests/Security/EscapeOutputUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ public function getErrorList( $testFile = '' ) {
654 => 1,
655 => 1,
657 => 1,
660 => 1,
661 => 2,
);

case 'EscapeOutputUnitTest.6.inc':
Expand Down
9 changes: 8 additions & 1 deletion WordPress/Tests/Security/NonceVerificationUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ function function_containing_nested_enum_with_nonce_check() {
wp_verify_nonce( sanitize_text_field( wp_unslash( $_POST['my_nonce'] ) ), 'the_nonce' );
}
}

echo $_POST['foo']; // Bad.
}

Expand All @@ -486,3 +486,10 @@ enum MyEnum {
echo $_POST['foo']; // OK.
}
}
// phpcs:set WordPress.Security.NonceVerification customNonceVerificationFunctions[] MY_nonce_check
// phpcs:set WordPress.Security.NonceVerification customUnslashingSanitizingFunctions[] do_sometHING
function foo_6() {
my_nonce_check( do_something( $_POST['tweet'] ) ); // OK.
}
// phpcs:set WordPress.Security.NonceVerification customUnslashingSanitizingFunctions[]
// phpcs:set WordPress.Security.NonceVerification customNonceVerificationFunctions[]
29 changes: 29 additions & 0 deletions WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -500,3 +500,32 @@ function test_in_match_condition_is_regarded_as_comparison() {
};
}
}

/*
* Test using custom properties, setting & unsetting (resetting).
*/
function test_this() {
if ( ! isset( $_POST['abc_field'] ) ) {
return;
}

$abc = sanitize_color( wp_unslash( $_POST['abc_field'] ) ); // Bad x1 - sanitize.

// phpcs:set WordPress.Security.ValidatedSanitizedInput customSanitizingFunctions[] sanitize_color,sanitize_TWITTER_handle

$abc = sanitize_color( wp_unslash( $_POST['abc_field'] ) ); // OK.
$abc = sanitize_facebook_id( wp_unslash( $_POST['abc_field'] ) ); // Bad x1 - sanitize.
$abc = sanitize_twitter_handle( $_POST['abc_field'] ); // Bad x1 - unslash.

// phpcs:set WordPress.Security.ValidatedSanitizedInput customSanitizingFunctions[] sanitize_color,sanitize_facebook_ID
// phpcs:set WordPress.Security.ValidatedSanitizedInput customUnslashingSanitizingFunctions[] sanITize_twitter_handle

$abc = sanitize_color( wp_unslash( $_POST['abc_field'] ) ); // OK.
$abc = sanitize_facebook_id( wp_unslash( $_POST['abc_field'] ) ); // OK.
$abc = sanitize_twitter_handle( $_POST['abc_field'] ); // OK.

// phpcs:set WordPress.Security.ValidatedSanitizedInput customSanitizingFunctions[]
// phpcs:set WordPress.Security.ValidatedSanitizedInput customUnslashingSanitizingFunctions[]

$abc = sanitize_twitter_handle( $_POST['abc_field'] ); // Bad x2, sanitize + unslash.
}
4 changes: 4 additions & 0 deletions WordPress/Tests/Security/ValidatedSanitizedInputUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ public function getErrorList( $testFile = '' ) {
497 => 1,
498 => 1,
499 => 3,
512 => 1,
517 => 1,
518 => 1,
530 => 2,
);

case 'ValidatedSanitizedInputUnitTest.2.inc':
Expand Down