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

Nonce verifcation check false positives #759

Closed
DavidAnderson684 opened this issue Oct 29, 2024 · 2 comments
Closed

Nonce verifcation check false positives #759

DavidAnderson684 opened this issue Oct 29, 2024 · 2 comments

Comments

@DavidAnderson684
Copy link

DavidAnderson684 commented Oct 29, 2024

In lots of plugins which I'm checking, every nonce check throws two warnings which are false positives. Here's an example:

if (empty($_POST) || !isset($_POST['_wpnonce']) || !wp_verify_nonce($_POST['_wpnonce'], 'my_nonce') die('Security check');

This will result in:

WARNING 	WordPress.Security.ValidatedSanitizedInput.MissingUnslash  	$_POST['_wpnonce'] not unslashed before sanitization. Use wp_unslash() or similar 
WARNING 	WordPress.Security.ValidatedSanitizedInput.InputNotSanitized 	Detected usage of a non-sanitized input variable: $_POST['_wpnonce'] 

The column numbers given for both warnings are for the wp_verify_nonce() call.

  1. Anything going to wp_verify_nonce() should be exempted from requiring unslashing. Nonces don't include slashes, and if a logged-in user decided to throw some in the only result would be that he failed the nonce check anyway.

  2. Similarly, anything going to wp_verify_nonce() doesn't need sanitising. Again, if the logged-in user decides to throw in characters that aren't found in nonces usually, he'll just fail the nonce check.

@davidperezgar
Copy link
Member

We know that nonces has a lot of false positives. That's why it's a warning instead of error. This check is running WPCS, so it could be a good idea to create an issue in WPCS.

@swissspidy
Copy link
Member

Agreed. Therefore closing as this would need to be handled upstream.

@swissspidy swissspidy closed this as not planned Won't fix, can't repro, duplicate, stale Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants