fix: fail closed on registration nonce + safe redirect (CSRF session fixation) - #1927
Open
arifulhoque7 wants to merge 1 commit into
Open
fix: fail closed on registration nonce + safe redirect (CSRF session fixation)#1927arifulhoque7 wants to merge 1 commit into
arifulhoque7 wants to merge 1 commit into
Conversation
process_registration() called wp_verify_nonce() in void context, so any non-empty _wpnonce passed and registration proceeded on any URL — a CSRF that could create an account and, with autologin on, fixate the victim's session. Fail closed: return early when the nonce is missing or invalid. Also switch the post-registration redirect from wp_redirect() to wp_safe_redirect() so an attacker-supplied redirect_to cannot bounce the victim to an arbitrary external host (matches the existing safe redirect later in the same method). Reported by Yaswanth Reddy Sunkara. Closes weDevsOfficial/wpuf-pro#1654.
WalkthroughRegistration now rejects failed nonce verification before processing and uses ChangesRegistration Security
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Vulnerability
[WPScan] Registration nonce check has no effect, leading to login session fixation via CSRF (CVSS 4.3).
Registration::process_registration()(hooked oninit, so it fires on any URL) calledwp_verify_nonce()in void context — nothing branched on the result, so any non-empty_wpnoncepassed and registration proceeded. Withautologin_after_registrationon (default), the handler cleared the visitor's auth cookie and set a new one, binding their browser to an attacker-created account. The post-registration redirect also usedwp_redirect()on$_POST['redirect_to']with no host allowlist, redirecting the victim off-site in the same request.Fix (minimal, no behavior change for legit flows)
_wpnonceis missing or invalid. The real form (templates/registration-form.php) already mintswp_nonce_field( 'wpuf_registration_action' ), so valid registrations are unaffected.wp_redirect()→wp_safe_redirect()for the post-registration redirect, matching the already-safe redirect later in the same method. Internal redirects keep working; external hosts are blocked.Verified — before / after (local WP, real HTTP requests)
Invalid nonce (the PoC form):
HTTP/2 302→location: https://attacker.example.com/, `set-cookie: wordpress_logged_in_…=csrf_probe_userHTTP/2 200, no redirect, no login cookie, user not createdValid nonce (legit registration form), on the patched code:
HTTP/2 302→location: https://wps-site.test/registration/?success=yes, user created — legit signup unaffected, and the redirect stays on-site.Reported by external security researcher Yaswanth Reddy Sunkara — please credit in the changelog/advisory.
Closes weDevsOfficial/wpuf-pro#1654.