Don't re-apply persistent credential headers across cross-origin redirects#433
Open
oalders wants to merge 1 commit into
Open
Don't re-apply persistent credential headers across cross-origin redirects#433oalders wants to merge 1 commit into
oalders wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #433 +/- ##
==========================================
+ Coverage 89.84% 89.95% +0.10%
==========================================
Files 3 3
Lines 857 866 +9
Branches 226 227 +1
==========================================
+ Hits 770 779 +9
Misses 37 37
Partials 50 50 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
a3e0676 to
9c133d0
Compare
Persistent headers set via add_header() (Authorization, Proxy-Authorization and Cookie) were re-applied by _modify_request() to cross-origin redirect targets, leaking them to a different origin (the CVE-2018-1000007 class of leak). _modify_request() now detects a cross-origin redirect hop using the response LWP::UserAgent threads into request(), and removes the credential headers from the request outright on such a hop. Removing them (rather than only declining to re-apply our own) means the fix does not depend on the LWP version: older LWP releases clone these headers forward instead of stripping them. The cookie jar is unaffected, since it sets its domain-scoped Cookie header later in prepare_request. allow_credentialed_redirects opts out. Mech now owns that option (stored in the same hash slot LWP::UserAgent uses) so the opt-out is honoured on every LWP version, including releases that predate LWP's own cross-origin strip. A new _is_cross_origin() compares scheme and host_port (after canonical), so case-only and default-port differences do not count as cross-origin. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9c133d0 to
c1d0b83
Compare
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.
Summary
Headers set via
add_header()— notablyAuthorization— were re-applied by_modify_request()to cross-origin redirect targets, undoing the credential strippingLWP::UserAgentperforms on its own cross-origin redirects._modify_request()now detects a cross-origin redirect hop using the responseLWP::UserAgentthreads intorequest(), and skips re-applyingAuthorization/Proxy-Authorizationto a different origin. This mirrors LWP's own redirect handling. Setallow_credentialed_redirectsto a true value to opt back in.Details
_is_cross_origin()compares scheme andhost_portof the previous hop and the new request (aftercanonical, so case-only and default-port differences don't count as cross-origin).request()passes the LWP-threaded triggering response into_modify_request()so the cross-origin check only fires on redirect/auth re-entry, never on a normal first request.allow_credentialed_redirectsopt-out, preserve the header exactly as before.Tests
t/cross-origin-redirect.tdrives LWP's real redirect loop over a mocked transport and asserts, table-driven:AuthorizationandProxy-Authorizationallow_credentialed_redirectsopts back in (preserves)🤖 Generated with Claude Code