Always sanitize the first parameter of wp_verify_nonce

[davidperez]

We ask developers to sanitize and unslash the first parameter of wp_verify_nonce.
Wouldn't it be easier if that piece of repetitive code were not necessary? In the Plugins Team, we often detect that developers forget to do this.

I’m also thinking about the wider community. Many developers may be using wp_verify_nonce incorrectly, without sanitizing it. By handling this automatically, we could prevent security leaks and encourage cleaner code.

This is the correct way to use it right now:

<?php
<?php
wp_verify_nonce( sanitize_text_field( wp_unslash( $_POST['plugin_slug_nonce_field'] ) ), 'plugin_slug_action_nonce' ) )

And this is the ideal way, with sanitization handled inside the function:

<?php
<?php
wp_verify_nonce( $_POST['plugin_slug_nonce_field'], 'plugin_slug_action_nonce' ) )

Does this make it cleaner and more straightforward?

[rollybueno]

Personally, I don't think the $nonce param needs sanitizing. There's no input/output on wp_verify_nonce(), no database saving and no actual output display. It simply uses the $nonce value inside hash_equals, which should returns boolean, but we use 1 and 2. There's no possible security threat in that matter.

However, if this is what the Plugins Team prefers on the submission guidelines, then we can add it after the string type casting.

You can see the function here: https://core.trac.wordpress.org/browser/tags/6.8.2/src/wp-includes/pluggable.php#L2369

[rishabhwp]

Even simply replacing:

$nonce = (string) $nonce;

with

$nonce = sanitize_text_field( $nonce );

would work, as sanitize_text_field() internally performs the type casting (see formatting.php#L5631).

I’m happy to create a patch if that sounds reasonable.

[pruthviraj07]

Hi everyone,

From my understanding, the proposal here is to ensure that the first parameter of wp_verify_nonce() is automatically sanitized internally, so plugin and theme developers don’t need to repeatedly write:

wp_verify_nonce( sanitize_text_field( wp_unslash( $_POST['field'] ) ), 'action' );

A possible change could be updating the function like so:

$nonce = sanitize_text_field( (string) $nonce );

instead of only casting to string.

This way, sanitization becomes consistent across the ecosystem, and the risk of developers forgetting to sanitize is reduced.

I’d like to work on preparing a patch with this modification. Before I proceed, I’d love to hear feedback from core committers:

• Do we see this as a safe enhancement for backwards compatibility?
• Should sanitization be applied unconditionally, or only when the $nonce is passed from $_REQUEST/$_POST contexts?

Thanks! Happy to iterate based on guidance.

[SirLouen]

Replying to rollybueno:

Personally, I don't think the $nonce param needs sanitizing. There's no input/output on wp_verify_nonce(), no database saving and no actual output display. It simply uses the $nonce value inside hash_equals, which should returns boolean, but we use 1 and 2. There's no possible security threat in that matter.

A malformed nonce gets direct access to the action hook within wp_verify_nonce. This has probably become a CVE for other plugins at some point in the past that could coexist with other plugins that don't sanitize and call to this hook, most likely. Ultimately we could say that it's their fault for not sanitizing the incoming nonce parameter depending on the use they make of it…

And similarly to what you have observed, who could care to sanitize the nonce? I'm not sure either why the core/plugins members should take responsibility for sanitizing this nonce either. If anyone is going to play further with the hook, they should do their duty.

[peterwilsoncc]

@davidperez I'm afraid WordPress can't unslash the parameter sent to wp_verify_nonce() for a couple of reasons:

• it assumes that the parameter is a form input, which is usually but not always the case
• it could break sites with custom implementations of nonces by double unslashing their data. For example the form input pens\and\pencils would become pensandpencils after double unslashing.

For similar reasons, sanitization within the function could also become problematic. WordPress does handle double sanitization but doing so makes presumptions about the implementation.

As to whether the coding standards should require sanitization, there's a big long discussion in the ​WordPress Coding Standards issue tracker. Feel free to share all your thoughts there.

As this change can't be made without breaking backward compatibility, I'm going to close the ticket as wontfix.

[davidperez]

Hello,
I understand the backwards compatibility for unslash, but not for sanitizing. We have the Plugin Check plugin to prevent developers from using the code incorrectly, so we could avoid double sanitization over time.

Thanks for your explanation.