Repository navigation
fix(transport): refuse cross-origin redirects in default HTTP clients (GO-2026-6911) #1192
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
Open
hymns
wants to merge
6
commits into
mark3labs:main
Choose a base branch
from
hymns:fix/same-origin-redirects
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
390448b
fix(transport): refuse cross-origin redirects in default HTTP clients
hymns 8ce69db
fix(transport): tighten origin checks and cover OAuth credential requ…
hymns 41a7375
test(transport): fix names of the dotted capital I cases
hymns 8d778b5
fix(transport): compare IP literals in canonical form
hymns 4c8da10
fix(transport): keep IPv6 zone case when comparing origins
hymns c7131c9
test(transport): cover RefreshToken against a cross-origin token redi…
hymns File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| package transport | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "net/url" | ||
| "strconv" | ||
| "strings" | ||
| ) | ||
|
|
||
| // ErrCrossOriginRedirect is returned when a server redirects a request to a | ||
| // different origin (scheme, host or port) and the redirect policy does not | ||
| // allow it. The redirected request would otherwise carry the configured | ||
| // headers, the OAuth bearer token and, for 307 and 308, the request body to a | ||
| // destination the server chose. | ||
| var ErrCrossOriginRedirect = errors.New("refusing cross-origin redirect") | ||
|
|
||
| // ErrInsecureRedirect is returned when a server redirects an https request to | ||
| // a plain http URL. | ||
| var ErrInsecureRedirect = errors.New("refusing redirect from https to http") | ||
|
|
||
| // ErrTooManyRedirects is returned when a redirect chain reaches maxRedirects. | ||
| var ErrTooManyRedirects = errors.New("too many redirects") | ||
|
|
||
| // maxRedirects matches the limit net/http applies when CheckRedirect is nil. | ||
| const maxRedirects = 10 | ||
|
|
||
| // newDefaultHTTPClient returns the HTTP client a transport uses when the | ||
| // caller does not supply one. It only follows same-origin redirects. | ||
| func newDefaultHTTPClient() *http.Client { | ||
| return &http.Client{CheckRedirect: sameOriginRedirectPolicy} | ||
| } | ||
|
|
||
| // sameOriginRedirectPolicy is an http.Client CheckRedirect function that | ||
| // follows a redirect only when the new URL has the origin of the request that | ||
| // started the chain, and at most maxRedirects times. | ||
| func sameOriginRedirectPolicy(req *http.Request, via []*http.Request) error { | ||
| if len(via) >= maxRedirects { | ||
| return fmt.Errorf("%w: stopped after %d redirects", ErrTooManyRedirects, maxRedirects) | ||
| } | ||
| if len(via) == 0 { | ||
| return nil | ||
| } | ||
| from := via[0].URL | ||
| if !sameOrigin(from, req.URL) { | ||
| return fmt.Errorf("%w: %s://%s to %s://%s", | ||
| ErrCrossOriginRedirect, from.Scheme, from.Host, req.URL.Scheme, req.URL.Host) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // oauthRedirectPolicy is the CheckRedirect function of the OAuth handler's | ||
| // default client. Requests that carry credentials (token, refresh and client | ||
| // registration requests, which are not GET or HEAD) follow only same-origin | ||
| // redirects. Metadata discovery requests, which carry no credentials, may be | ||
| // redirected to another origin but never from https to http. | ||
| func oauthRedirectPolicy(req *http.Request, via []*http.Request) error { | ||
| if len(via) == 0 { | ||
| return nil | ||
| } | ||
| first := via[0] | ||
| if first.Method != http.MethodGet && first.Method != http.MethodHead { | ||
| return sameOriginRedirectPolicy(req, via) | ||
| } | ||
| if len(via) >= maxRedirects { | ||
| return fmt.Errorf("%w: stopped after %d redirects", ErrTooManyRedirects, maxRedirects) | ||
| } | ||
| prev := via[len(via)-1].URL | ||
| if asciiLower(prev.Scheme) == "https" && asciiLower(req.URL.Scheme) != "https" { | ||
| return fmt.Errorf("%w: %s to %s", ErrInsecureRedirect, prev.Redacted(), req.URL.Redacted()) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // sameOrigin reports whether a and b have the same scheme, host and port. | ||
| func sameOrigin(a, b *url.URL) bool { | ||
| return origin(a) == origin(b) | ||
| } | ||
|
|
||
| // origin returns a normalized scheme://host:port string. The scheme and host | ||
| // are lowercased (ASCII only), a missing port becomes the scheme's default, | ||
| // and a numeric port loses leading zeros, so ":0443" and ":443" compare equal | ||
| // as net/http dials them. | ||
| func origin(u *url.URL) string { | ||
| scheme := asciiLower(u.Scheme) | ||
| port := u.Port() | ||
| if n, err := strconv.ParseUint(port, 10, 16); err == nil { | ||
| port = strconv.FormatUint(n, 10) | ||
| } | ||
| if port == "" { | ||
| port = defaultPort(scheme) | ||
| } | ||
| return scheme + "://[" + asciiLower(u.Hostname()) + "]:" + port | ||
| } | ||
|
|
||
| // asciiLower lowercases only ASCII letters. Unicode case mapping must not be | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| // used on hosts: strings.ToLower maps "İ" to "i" and strings.EqualFold matches | ||
| // "K" (Kelvin sign) with "k", while net/http dials those as different hosts. | ||
| func asciiLower(s string) string { | ||
| return strings.Map(func(r rune) rune { | ||
| if 'A' <= r && r <= 'Z' { | ||
| return r + ('a' - 'A') | ||
| } | ||
| return r | ||
| }, s) | ||
| } | ||
|
|
||
| func defaultPort(scheme string) string { | ||
| switch scheme { | ||
| case "http": | ||
| return "80" | ||
| case "https": | ||
| return "443" | ||
| } | ||
| return "" | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.