fix: do not forward Authorization/Cookie to subdomains on redirect#63256
fix: do not forward Authorization/Cookie to subdomains on redirect#63256mohammadmseet-hue wants to merge 2 commits intodart-lang:mainfrom
Conversation
`_HttpClient.shouldCopyHeaderOnRedirect` (the policy that decides which headers carry over a 3xx redirect) returns true for any redirect to a subdomain of the original host, BEFORE consulting the `authorization` / `cookie` denylist. As a result, a redirect from `https://api.example.com` to `https://attacker-uploads.example.com` forwards the victim's `Authorization: Bearer <token>` and host-only `Cookie` to the subdomain — even though the subdomain may be attacker-controlled. Real-world attacker scenarios where this is exploitable: - Shared-apex platforms: user-content subdomains on a single apex (e.g. `*.example.com` user pages, `*.s3.amazonaws.com` buckets, Heroku/GitHub-Pages-style platforms with custom domains). - Open-redirect gadgets that 302 to `attacker-foo.example.com`. - HTTP servers compromised at a single subdomain (e.g. a CDN edge). This contradicts the standard credential-handling rule applied by every mainstream HTTP client (curl/libcurl, Python requests, browser fetch, Go net/http): credentials MUST be stripped on ANY host change, including subdomains. RFC 6265 host-only cookies (no Domain attribute) also MUST NOT be sent to subdomains. Live reproduction (logic-level, against current `main`): $ docker run --rm -v /tmp:/app -w /app dart:stable dart verify_fix.dart http://victim.com/ -> http://sub.victim.com/ [authorization] = COPIED (LEAK) http://victim.com/ -> http://sub.victim.com/ [cookie] = COPIED (LEAK) A full HTTP test against an unpatched dart binary requires building from `main`; the standalone trace above shows the policy decision happens before the body of `_openUrlFromRequest` ever sends the follow-up request, so the leak is direct. Fix --- Re-order the policy: 1. If the header is `authorization`, `www-authenticate`, `cookie`, or `cookie2`, only copy it when scheme:host:port is identical (sameHost). Subdomains do NOT receive credentials. 2. Other headers may be copied to the same origin OR to a subdomain of the original host (preserves legitimate behaviour for tracing / user-agent / accept-language headers). 3. Cross-origin redirects continue to strip everything. Test plan --------- The standalone trace `verify_fix.dart` in this PR description shows the four cases (subdomain redirect / same host / different host / subdomain-to-apex) and the patched output for each. After the fix: subdomain redirect authorization = STRIPPED (was COPIED) subdomain redirect cookie = STRIPPED (was COPIED) subdomain redirect x-trace = COPIED (unchanged) same host authorization = COPIED (unchanged — legit) different host authorization = STRIPPED (unchanged) subdomain → apex authorization = STRIPPED (unchanged) Adding a test in `tests/standalone/io/http_redirect_*_test.dart` in a follow-up if reviewer prefers — wanted to surface the fix quickly given the credential-leak class.
|
Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at: https://dart-review.googlesource.com/c/sdk/+/498424 Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly. Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR). |
|
CL has new comments, please view and respond to them in Gerrit. If a reviewer requested changes, push new commits to this PR and it will be automatically copied to Gerrit. After that you can mark reviewer comments as resolved in Gerrit and request another round of reviews. Note: when you add comments in Gerrit they only become visible after you send them by clicking Reply and Send. |
- Sensitive headers (authorization/cookie/cookie2/www-authenticate) now strip on subdomain redirects; non-sensitive headers preserve existing behaviour (always follow). Old code returned false on cross-origin for non-sensitive too — that was a regression. - testShouldCopyHeadersOnRedirect: flip subdomain authorization assertion to expect strip; add cookie/cookie2/www-authenticate strip-on-subdomain cases and same-host preserve cases; add deeper-subdomain attacker case.
|
https://dart-review.googlesource.com/c/sdk/+/498424 has been updated with the latest commits from this pull request. |
|
CL has new comments, please view and respond to them in Gerrit. If a reviewer requested changes, push new commits to this PR and it will be automatically copied to Gerrit. After that you can mark reviewer comments as resolved in Gerrit and request another round of reviews. Note: when you add comments in Gerrit they only become visible after you send them by clicking Reply and Send. |
|
https://dart-review.googlesource.com/c/sdk/+/498424 has been updated with the latest commits from this pull request. |
|
CL has new comments, please view and respond to them in Gerrit. If a reviewer requested changes, push new commits to this PR and it will be automatically copied to Gerrit. After that you can mark reviewer comments as resolved in Gerrit and request another round of reviews. Note: when you add comments in Gerrit they only become visible after you send them by clicking Reply and Send. |
2 participants
Summary
_HttpClient.shouldCopyHeaderOnRedirect(the policy that decides which headers carry over a 3xx redirect) returns true for any redirect to a subdomain of the original host, before consulting theauthorization/cookiedenylist. A redirect fromhttps://api.example.comtohttps://attacker-uploads.example.comforwards the victim'sAuthorization: Bearer <token>and host-onlyCookieto the subdomain — even though that subdomain may be attacker-controlled.Why subdomains can be attacker-controlled
*.example.comuser pages,*.s3.amazonaws.combuckets, Heroku / GitHub Pages style platforms with custom domains).attacker-foo.example.com.This contradicts the standard credential-handling rule applied by every mainstream HTTP client (curl/libcurl, Python
requests, browserfetch, Gonet/http): credentials MUST be stripped on any host change, including subdomains. RFC 6265 host-only cookies (noDomainattribute) also MUST NOT be sent to subdomains.Live reproduction
Standalone trace against current
main'sshouldCopyHeaderOnRedirect:A full integration test against an unpatched dart binary requires building from
main; the trace above demonstrates the policy decision directly — the function returnstruefor sensitive headers before the body of_openUrlFromRequestever sends the follow-up request, so the leak is direct.Fix
Re-order the policy:
authorization/www-authenticate/cookie/cookie2: copy only when scheme:host:port is identical. Subdomains do NOT receive credentials.+27 / -7lines.Test plan
The standalone trace
verify_fix.dartshows the four cases (subdomain / same host / different host / subdomain-to-apex) and the patched output:Adding a test in
tests/standalone/io/http_redirect_*_test.dartin a follow-up if reviewer prefers — wanted to surface the fix quickly given the credential-leak class.Related
This builds on #63133 (param-swap fix). The previous PR fixed the call-site argument order so the function got the right URLs; this PR fixes the function itself so the policy aligns with browser / curl / requests / fetch convention.