Repository navigation
One-time OAuth login nonce for shared-playground logins (cubic P1) - #36
Merged
Merged
Conversation
Bare playground ids are public, so the cookie==state CSRF check was vacuous on the member login path: an attacker could authorize their own account and lure a member's browser to the callback, linking the attacker's token into the victim's member slot. Logins started from a bare id now carry an unguessable nonce echoed in an HttpOnly cookie; the callback requires the two to match and consumes the nonce. Legacy/bearer states are unguessable and unchanged. Tests cover the attack (bare state without nonce rejected even for members), the fixed flow, nonce single-use, and legacy compat.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
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.
Addresses the P1 from cubic's post-merge review of #35: bare playground ids are public, so the cookie==state CSRF check was vacuous on the member login path. Logins started from a bare id now carry an unguessable nonce echoed in an HttpOnly cookie; the callback requires the two to match and consumes the nonce. Legacy/bearer states unchanged. Tests in tests/sharing-canonical.test.ts cover the attack, the fixed flow, nonce single-use, and legacy compat.
Summary by cubic
Fixes a login CSRF vulnerability where shared-playground logins started from a bare (public) playground id could be replayed cross-browser, letting an attacker link their own GitHub account into a victim's member slot.
Logins from a bare id now carry an unguessable one-time nonce in an
HttpOnlycookie; the callback requires the nonce to match the OAuthstateand expires the cookie once the login is consumed, so a captured state can't be replayed. Legacy and bearer states are already unguessable and are unchanged.Tests cover the attack (bare state without a nonce rejected even for valid members), the fixed flow, nonce single-use, and legacy compatibility.
Written for commit 8cd4b57. Summary will update on new commits.