-
Notifications
You must be signed in to change notification settings - Fork 82
Open
Milestone
Description
We need a CodeSniffer sniff that detects buggy and insecure usage patterns of WordPress's wp_verify_nonce() function. It will check for logical errors that could allow code to execute even when nonce verification fails.
The Problems to detect
1. Unsafe Negated Nonce with AND (&&) Operator
Problematic Pattern:
if ( $some_condition && !wp_verify_nonce($nonce, 'my_action') ) {
wp_die('Security check failed');
}Why it's dangerous:
- If
$some_conditionis false, PHP's short-circuit evaluation means the nonce check never runs - The code continues executing without any nonce verification
- Attacker can bypass security by making
$some_conditionfalse
What the sniff reports: UnsafeVerifyNonceNegatedAnd
Safe alternative:
if ( !wp_verify_nonce($nonce, 'my_action') ) {
wp_die('Security check failed');
}2. Unsafe Non-Negated Nonce with OR (||) Operator
Problematic Pattern:
if ( $some_condition || wp_verify_nonce($nonce, 'my_action') ) {
// Do something safe
} else {
wp_die('Access denied');
}Why it's dangerous:
- If
$some_conditionis true, the nonce check never runs (short-circuit evaluation) - Code in the
ifblock executes without nonce verification - Attacker can bypass security by making
$some_conditiontrue
What the sniff reports: UnsafeVerifyNonceElse
Safe alternative:
if ( !wp_verify_nonce($nonce, 'my_action') ) {
wp_die('Security check failed');
}
// Continue with safe code3. Unconditional Call (Not in Conditional)
Problematic Pattern:
wp_verify_nonce($nonce, 'my_action'); // Does nothing!
// Code continues regardless
update_option('important_setting', $_POST['value']);Why it's dangerous:
wp_verify_nonce()only returns true/false, it doesn't stop execution- Without checking the return value, the nonce verification is completely ignored
- This is likely a confusion with
check_admin_referer(), which DOES exit on failure
What the sniff reports: UnsafeVerifyNonceStatement
Correct usage:
// Option 1: Use check_admin_referer() which exits on failure
check_admin_referer('my_action');
// Option 2: Check the return value
if ( !wp_verify_nonce($nonce, 'my_action') ) {
wp_die('Security check failed');
}This sniff helps prevent CSRF vulnerabilities by catching these subtle but dangerous patterns that developers might not notice.
This check is based in Code Analysis check from WPORG.
Metadata
Metadata
Assignees
Labels
No labels