Skip to content

Commit 7b18749

Browse files
committed
Sniff::has_nonce_check(): allow for comparing a variable before nonce check
This builds onto the similar changes made for the `ValidatedSanitizedInput` sniff in ... This fixes false positives as reported in 1114 and 1506. Note: it is not currently checked that the nonce check is done within the same conditional scope as the comparison. Just that it is done within the same _function_ scope. Includes unit tests. Fixes 1114 Fixes 1506
1 parent 0ef9e85 commit 7b18749

File tree

2 files changed

+39
-0
lines changed

2 files changed

+39
-0
lines changed

WordPress/Sniff.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,6 +1440,8 @@ protected function has_nonce_check( $stackPtr ) {
14401440
$allow_nonce_after = false;
14411441
if ( $this->is_in_isset_or_empty( $stackPtr )
14421442
|| $this->is_in_type_test( $stackPtr )
1443+
|| $this->is_comparison( $stackPtr )
1444+
|| $this->is_in_array_comparison( $stackPtr )
14431445
) {
14441446
$allow_nonce_after = true;
14451447
}

WordPress/Tests/Security/NonceVerificationUnitTest.inc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,3 +209,40 @@ function skip_over_nested_constructs_2() {
209209
}
210210
};
211211
}
212+
213+
// Issue #1506
214+
function allow_for_compare_before_noncecheck() {
215+
if (
216+
'newsletter_sign_up' === $_POST['action'] && // OK.
217+
wp_verify_nonce( $_POST['newsletter_nonce'] )
218+
) {}
219+
}
220+
221+
// Issue #1114
222+
function allow_for_nonce_check_within_switch() {
223+
if ( ! isset( $_REQUEST['action'] ) ) {
224+
return;
225+
}
226+
227+
switch ( $_REQUEST['action'] ) { // OK.
228+
case 'foo':
229+
check_admin_referer( 'foo' );
230+
break;
231+
case 'bar':
232+
check_admin_referer( 'bar' );
233+
break;
234+
}
235+
}
236+
237+
function allow_for_array_compare_before_noncecheck() {
238+
if ( array_search( array( 'subscribe', 'unsubscribe', $_POST['action'], true ) // OK.
239+
&& wp_verify_nonce( $_POST['newsletter_nonce'] )
240+
) {}
241+
}
242+
243+
function allow_for_array_comparison_in_condition() {
244+
if ( in_array( $_GET['action'], $valid_actions, true ) ) { // OK.
245+
check_admin_referer( 'foo' );
246+
foo();
247+
}
248+
}

0 commit comments

Comments
 (0)