-
Notifications
You must be signed in to change notification settings - Fork 440
fix: prevent OAuth parameter injection via returnTo (#2381) #2413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- URL encode returnTo parameter to prevent injection of OAuth parameters - Add comprehensive unit tests for returnTo encoding scenarios - Tests cover basic encoding, OAuth param injection attempts, and edge cases Co-authored-by: Simen A. W. Olsen <my@simen.io>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2413 +/- ##
=======================================
Coverage 90.39% 90.39%
=======================================
Files 39 39
Lines 4488 4488
Branches 912 911 -1
=======================================
Hits 4057 4057
Misses 425 425
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I need to correct an attribution error in this PR. The original security fix was contributed by Joshua Rogers (@MegaManSec) in PR #2381, not "Simen A. W. Olsen" as incorrectly stated in the commit message. We will update the changelog accordingly. |
Credit Joshua Rogers (@MegaManSec) as the original author who discovered and fixed the OAuth parameter injection vulnerability in PR #2381. This corrects an attribution error in PR #2413 where the commit message incorrectly credited a different person.
Description
This PR addresses a security issue where OAuth parameters could be injected via the
returnToparameter inwithPageAuthRequired.Changes
returnToparameter to prevent injection of OAuth parameters like&prompt=loginRelated PRs
Testing
All existing tests pass, plus 3 new test cases specifically for this issue:
Note: This PR originally contained an attribution error in the commit message. The security issue was discovered and fixed by Joshua Rogers (@MegaManSec) in #2381. See correction comment below.