Repository navigation
Conversation
performGitHubWebOAuth() never passed a state option to getWebFlowAuthorizationUrl(), and handleCallback() never checked one on the gitify://oauth callback either, so any authorization code arriving through that custom protocol was accepted and exchanged for a token regardless of where it came from. An attacker who completes GitHub's OAuth authorize step for their own account can get the resulting code delivered to a victim's running Gitify instance via a crafted gitify://oauth?code=...&type=oauth link (custom protocol handlers can be triggered from a webpage, email, or another local app), silently binding the victim's client to the attacker's GitHub account instead of the victim's own. Generate a per-attempt random state value, pass it through to the authorization URL, and require the callback's state to match before treating a code as belonging to that login attempt. A callback with a missing or mismatched state is logged and ignored rather than accepted, so a still-pending legitimate login isn't torn down by an attacker's delivery attempt. Updated the existing web-flow success test to supply the now-required state on its simulated callback, and added a regression test confirming a mismatched or missing state does not resolve the login.
Member
|
Thanks for flagging @carfeii. I've used this as a base to fix a few other edge cases in the OAuth state handling. Appreciate it! |
Member
|
Actually, we might be able to remove this oauth method completely. #3449 |
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
2 participants
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.
performGitHubWebOAuth() never passed a state option to getWebFlowAuthorizationUrl(), and handleCallback() never checked one on the gitify://oauth callback either, so any authorization code arriving through that custom protocol was accepted and exchanged for a token regardless of where it came from. An attacker who completes GitHub's OAuth authorize step for their own account can get the resulting code delivered to a victim's running Gitify instance via a crafted gitify://oauth?code=...&type=oauth link (custom protocol handlers can be triggered from a webpage, email, or another local app), silently binding the victim's client to the attacker's GitHub account instead of the victim's own.
This generates a per-attempt random state value, passes it through to the authorization URL, and requires the callback's state to match before treating a code as belonging to that login attempt. A callback with a missing or mismatched state is logged and ignored rather than accepted, so a still-pending legitimate login isn't torn down by an attacker's delivery attempt.
Updated the existing web-flow success test to supply the now-required state on its simulated callback, and added a regression test confirming a mismatched or missing state does not resolve the login. Full test suite run clean (10/10) in an isolated environment.