Skip to content

Commit d0c2fb0

Browse files
rodrigoprimojrfnl
andcommitted
Apply suggestions from code review
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
1 parent 6d8f58a commit d0c2fb0

2 files changed

Lines changed: 69 additions & 28 deletions

File tree

WordPress/Tests/Security/NonceVerificationUnitTest.1.inc

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -526,21 +526,29 @@ function different_function_name() {
526526
// phpcs:set WordPress.Security.NonceVerification customNonceVerificationFunctions[]
527527

528528
function test_fully_qualified_call_to_global_nonce_verification_function() {
529-
if ( ! IS_NUMERIC( $_POST['foo'] ) ) {
529+
if ( ! IS_NUMERIC( $_POST['foo'] ) ) { // OK.
530530
return;
531531
}
532532

533-
\wp_verify_nonce( 'some_action' ); // OK.
533+
\wp_verify_nonce( 'some_action' );
534+
}
535+
536+
function test_namespace_relative_call_to_global_nonce_verification_function() {
537+
if ( ! IS_NUMERIC( $_POST['foo'] ) ) { // Bad, but should become ok once the sniff is able to resolve relative namespaces.
538+
return;
539+
}
540+
541+
namespace\wp_verify_nonce( 'some_action' );
534542
}
535543

536544
function test_namespaced_calls_to_incorrect_functions() {
537-
if ( ! is_numeric( $_POST['foo'] ) ) {
545+
if ( ! is_numeric( $_POST['foo'] ) ) { // Bad - none of the below are the WP global functions, so no nonce verification.
538546
return;
539547
}
540548

541-
MyNamespace\wp_verify_nonce( 'some_action' ); // Bad.
542-
\MyNamespace\check_admin_referer( 'some_action' ); // Bad.
543-
namespace\check_ajax_referer( 'some_action' ); // Bad, but should become ok once the sniff is able to resolve relative namespaces.
549+
MyNamespace\wp_verify_nonce( 'some_action' );
550+
\MyNamespace\check_admin_referer( 'some_action' );
551+
namespace\Sub\check_ajax_referer( 'some_action' );
544552
}
545553

546554
function allow_fully_qualified_key_exists_functions() {
@@ -556,13 +564,21 @@ function allow_fully_qualified_key_exists_functions_with_mixed_case() {
556564
return;
557565
}
558566

559-
\WP_VERIFY_NONCE( 'some_action' );
567+
wp_verify_nonce( 'some_action' );
568+
}
569+
570+
function allow_namespace_relative_call_to_global_key_exists_functions() {
571+
if ( namespace\array_key_exists('foo', $_POST) === false ) { // Bad, but should become ok once the sniff is able to resolve relative namespaces.
572+
return;
573+
}
574+
575+
wp_verify_nonce( 'some_action' );
560576
}
561577

562578
function disallow_namespaced_key_exists_functions() {
563579
if ( MyNamespace\array_key_exists( 'foo', $_POST ) === false // Bad.
564580
|| \MyNamespace\key_exists( 'foo', $_POST ) === false // Bad.
565-
|| namespace\key_exists( 'foo', $_POST ) === false // Bad, but should become ok once the sniff is able to resolve relative namespaces.
581+
|| namespace\Sub\key_exists( 'foo', $_POST ) === false // Bad.
566582
) {
567583
return;
568584
}
@@ -575,26 +591,34 @@ function allow_fully_qualified_type_test_functions() {
575591
return;
576592
}
577593

578-
\wp_verify_nonce( 'some_action' ); // OK.
594+
\wp_verify_nonce( 'some_action' );
579595
}
580596

581597
function allow_fully_qualified_type_test_functions_uppercase() {
582-
if ( ! \IS_NUMERIC( $_POST['foo'] ) ) { // OK.
598+
if ( ! \IS_int( $_POST['foo'] ) ) { // OK.
583599
return;
584600
}
585601

586-
\wp_verify_nonce( 'some_action' ); // OK.
602+
\wp_verify_nonce( 'some_action' );
603+
}
604+
605+
function allow_namespace_relative_call_to_global_type_test_functions() {
606+
if ( ! namespace\is_numeric( $_POST['foo'] ) ) { // Bad, but should become ok once the sniff is able to resolve relative namespaces.
607+
return;
608+
}
609+
610+
\wp_verify_nonce( 'some_action' );
587611
}
588612

589613
function disallow_namespaced_type_test_functions() {
590614
if ( ! MyNamespace\is_bool( $_POST['foo'] ) // Bad.
591-
|| ! MyNamespace\is_object( $_POST['foo'] ) // Bad.
592-
|| ! namespace\is_string( $_POST['foo'] ) ) // Bad, but should become ok once the sniff is able to resolve relative namespaces.
615+
|| ! \MyNamespace\is_object( $_POST['foo'] ) // Bad.
616+
|| ! namespace\Sub\is_string( $_POST['foo'] ) ) // Bad.
593617
{
594618
return;
595619
}
596620

597-
\wp_verify_nonce( 'some_action' ); // OK.
621+
\wp_verify_nonce( 'some_action' );
598622
}
599623

600624
function allow_fully_qualified_array_comparison_functions() {
@@ -603,10 +627,16 @@ function allow_fully_qualified_array_comparison_functions() {
603627
}
604628
}
605629

630+
function allow_namespace_relative_call_to_global_array_comparison_functions() {
631+
if ( namespace\in_array( $_GET['action'], $valid_actions, true ) ) { // Bad, but should become ok once the sniff is able to resolve relative namespaces.
632+
check_admin_referer( 'foo' );
633+
}
634+
}
635+
606636
function disallow_namespaced_array_comparison_functions() {
607637
if ( MyNamespace\in_array( $_GET['action'], $valid_actions, true ) // Bad.
608638
|| \MyNamespace\array_search( array( 'subscribe', 'unsubscribe' ), $_GET['action'], true ) // Bad.
609-
|| namespace\array_keys( $_GET['actions'], 'my_action', true ) // Bad, but should become ok once the sniff is able to resolve relative namespaces.
639+
|| namespace\Sub\array_keys( $_GET['actions'], 'my_action', true ) // Bad.
610640
) {
611641
check_admin_referer( 'foo' );
612642
}
@@ -624,10 +654,16 @@ function allow_fully_qualified_unslashing_functions_mixed_case() {
624654
echo $var;
625655
}
626656

657+
function allow_namespace_relative_call_to_global_unslashing_functions() {
658+
$var = namespace\stripslashes_from_strings_only( $_POST['foo'] ); // Bad, but should become ok once the sniff is able to resolve relative namespaces.
659+
wp_verify_nonce( $var );
660+
echo $var;
661+
}
662+
627663
function disallow_namespaced_unslashing_functions() {
628664
$var = MyNamespace\stripslashes_from_strings_only( $_POST['foo'] ); // Bad.
629665
$var = \MyNamespace\stripslashes_deep( $_POST['foo'] ); // Bad.
630-
$var = namespace\wp_unslash( $_POST['foo'] ); // Bad, but should become ok once the sniff is able to resolve relative namespaces.
666+
$var = namespace\Sub\wp_unslash( $_POST['foo'] ); // Bad.
631667
wp_verify_nonce( $var );
632668
echo $var;
633669
}

WordPress/Tests/Security/NonceVerificationUnitTest.php

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,19 @@ public function getErrorList( $testFile = '' ) {
7676
478 => 1,
7777
524 => 2,
7878
537 => 1,
79-
563 => 1,
80-
564 => 1,
81-
565 => 1,
82-
590 => 1,
83-
591 => 1,
84-
592 => 1,
85-
628 => 1,
86-
629 => 1,
87-
630 => 1,
79+
545 => 1,
80+
571 => 1,
81+
579 => 1,
82+
580 => 1,
83+
581 => 1,
84+
606 => 1,
85+
614 => 1,
86+
615 => 1,
87+
616 => 1,
88+
658 => 1,
89+
664 => 1,
90+
665 => 1,
91+
666 => 1,
8892
);
8993

9094
case 'NonceVerificationUnitTest.2.inc':
@@ -117,9 +121,10 @@ public function getWarningList( $testFile = '' ) {
117121
return array(
118122
365 => 1,
119123
379 => 1,
120-
607 => 1,
121-
608 => 1,
122-
609 => 1,
124+
631 => 1,
125+
637 => 1,
126+
638 => 1,
127+
639 => 1,
123128
);
124129

125130
case 'NonceVerificationUnitTest.4.inc':

0 commit comments

Comments
 (0)