signup_nonce_check does not use wp_verify_nonce.

[herregroen]

Currently in multisite setups a nonce check is added to the singup form.

This check does not use the wp_verify_nonce function but instead creates a new nonce and expects an exact match. Due to the nature of wp_nonce_tick this means it's possible to generate nonces that are valid for only a few seconds twice a day.

The error message to try again could also use improvement. Most users will simply click the back button to try again, which will not generate a new nonce but simply restore the old form with the old nonce from browser memory.

[herregroen]

I've added a patch which uses the wp_verify_nonce function and expands the error message to include a link to the signup form. Clicking this link will make a new request which will generate a new nonce.

[flixos90]

Some thoughts:

• Do we need to add the link to the registration form? When clicking that, the user will have to re-enter their data. Without a link present, the user would likely hit the browser's back button, still having their data present.
• It's clear that the above isn't user-friendly in either case. I just noticed that when this error happens, the wp_die() is executed in the HTML content, causing ridiculously invalid markup. Since the method is hooked into the wpmu_validate_blog_signup and wpmu_validate_user_signup filters, both of which pass a $result array containing an errors key which is a WP_Error object, I think we should instead add that message to that WP_Error instance. This should cause it to be printed out in the content correctly, and the process will still fail. In that case, of course a link is no longer necessary anyway.

While the issue described under the second point is not caused by this patch, I think while we fix this one issue, we might as well fix the other as it's clearly broken.

[herregroen]

Replying to flixos90:

Some thoughts:

• Do we need to add the link to the registration form? When clicking that, the user will have to re-enter their data. Without a link present, the user would likely hit the browser's back button, still having their data present.
• It's clear that the above isn't user-friendly in either case. I just noticed that when this error happens, the wp_die() is executed in the HTML content, causing ridiculously invalid markup. Since the method is hooked into the wpmu_validate_blog_signup and wpmu_validate_user_signup filters, both of which pass a $result array containing an errors key which is a WP_Error object, I think we should instead add that message to that WP_Error instance. This should cause it to be printed out in the content correctly, and the process will still fail. In that case, of course a link is no longer necessary anyway.

While the issue described under the second point is not caused by this patch, I think while we fix this one issue, we might as well fix the other as it's clearly broken.

I added the link explicitly to avoid that behaviour. If the user goes back all his information is indeed still filled in, including the faulty nonce inside a hidden input. Meaning the same error will just occur regardless.

That said, I agree that simply returning an error is clearly the desired option. I'll update the patch to add an error instead. On error the signup form is output again in any case, including a new nonce, so the above is moot.

[flixos90]

43667.diff​ includes 4 unit tests calling wpmu_validate_blog_signup() and wpmu_validate_user_signup(), verifying that the hooked-in function works properly. I also made a minor change, making the error code more verbose with 'invalid_nonce' instead of 'nonce'.

[flixos90]

In 42976:

Multisite: Verify the signup nonce using wp_verify_nonce() in signup_nonce_check().

Prior to this change, the nonce passed from wp-signup.php was verified with a simple comparison. Furthermore in case of failures, wp_die() would be called right during the HTML markup being already printed. Now the error message is returned properly, modifying the WP_Error object in the passed $result.

Props herregroen.
Fixes #43667.