diff --git a/internal/httpx/httpx.go b/internal/httpx/httpx.go index 4ea20bc..cf30810 100644 --- a/internal/httpx/httpx.go +++ b/internal/httpx/httpx.go @@ -229,6 +229,17 @@ func parseHeaders(raw []string) (map[string]string, error) { return headers, nil } +// sensitiveHeaders mirrors net/http's own redirect header policy (see +// shouldCopyHeaderOnRedirect / copyHeaders in net/http/client.go): these are +// the headers the stdlib strips itself when a redirect crosses hosts, so +// injecting them back in at the transport layer would defeat that stripping. +var sensitiveHeaders = map[string]bool{ + "Cookie": true, + "Cookie2": true, + "Authorization": true, + "Www-Authenticate": true, +} + // roundTripper paces and decorates each request before delegating to base. type roundTripper struct { base *http.Transport @@ -240,14 +251,23 @@ type roundTripper struct { } func (rt *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + // req.Response is only populated when req was built to follow a redirect + // (see net/http's Request.Response doc comment); its Request is the prior + // hop, so a host mismatch here means this hop crossed hosts. + crossHostRedirect := req.Response != nil && req.Response.Request != nil && + req.Response.Request.URL.Host != req.URL.Host + // only set what the caller hasn't already; a scanner that explicitly sets a // header (e.g. an api key) must win over the global default. for key, value := range rt.headers { + if crossHostRedirect && sensitiveHeaders[http.CanonicalHeaderKey(key)] { + continue + } if req.Header.Get(key) == "" { req.Header.Set(key, value) } } - if rt.cookie != "" && req.Header.Get("Cookie") == "" { + if rt.cookie != "" && !crossHostRedirect && req.Header.Get("Cookie") == "" { req.Header.Set("Cookie", rt.cookie) } if rt.userAgent != "" && req.Header.Get("User-Agent") == "" { diff --git a/internal/httpx/httpx_test.go b/internal/httpx/httpx_test.go index ff187ee..8d845e7 100644 --- a/internal/httpx/httpx_test.go +++ b/internal/httpx/httpx_test.go @@ -715,3 +715,131 @@ func TestSleepCtxCancelled(t *testing.T) { t.Error("sleepCtx on a cancelled context should return its error, not block") } } + +// hostMappingTransport clones the default transport but resolves the given +// hostnames to the supplied local listener addresses, so a test can force a +// redirect across two genuinely distinct hostnames (not just distinct ports +// on 127.0.0.1, which net/http's redirect header stripping treats as the +// same host and never strips). +func hostMappingTransport(t *testing.T, hostToAddr map[string]string) *http.Transport { + t.Helper() + dialer := &net.Dialer{} + base := http.DefaultTransport.(*http.Transport).Clone() + base.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + host, _, err := net.SplitHostPort(addr) + if err != nil { + return nil, err + } + if mapped, ok := hostToAddr[host]; ok { + addr = mapped + } + return dialer.DialContext(ctx, network, addr) + } + return base +} + +// exercises the crossHostRedirect fix in roundTripper.RoundTrip (httpx.go). +func TestRedirectStripsSensitiveHeadersCrossHost(t *testing.T) { + var attackerSeen, targetSeen http.Header + + attacker := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + attackerSeen = r.Header.Clone() + })) + defer attacker.Close() + + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + targetSeen = r.Header.Clone() + if r.URL.Path == "/redirect" { + w.Header().Set("Location", "http://attacker.evil/loot") + w.WriteHeader(http.StatusFound) + } + })) + defer target.Close() + + base := hostMappingTransport(t, map[string]string{ + "target.test": target.Listener.Addr().String(), + "attacker.evil": attacker.Listener.Addr().String(), + }) + + rt := &roundTripper{ + base: base, + cookie: "session=secret", + headers: map[string]string{"Authorization": "Bearer topsecret"}, + userAgent: "sif-scanner", + } + client := &http.Client{Transport: rt} + + resp, err := client.Get("http://target.test/redirect") + if err != nil { + t.Fatalf("get: %v", err) + } + DrainClose(resp) + + if targetSeen == nil { + t.Fatal("target never received the initial request") + } + if targetSeen.Get("Cookie") != "session=secret" { + t.Errorf("target Cookie = %q, want it injected on the first hop", targetSeen.Get("Cookie")) + } + if targetSeen.Get("Authorization") != "Bearer topsecret" { + t.Errorf("target Authorization = %q, want it injected on the first hop", targetSeen.Get("Authorization")) + } + + if attackerSeen == nil { + t.Fatal("redirect never reached the attacker host") + } + if got := attackerSeen.Get("Cookie"); got != "" { + t.Errorf("attacker Cookie = %q, want empty (cross-host redirect must not leak it)", got) + } + if got := attackerSeen.Get("Authorization"); got != "" { + t.Errorf("attacker Authorization = %q, want empty (cross-host redirect must not leak it)", got) + } + if got := attackerSeen.Get("User-Agent"); got != "sif-scanner" { + t.Errorf("attacker User-Agent = %q, want sif-scanner (non-sensitive headers still inject)", got) + } +} + +// TestRedirectPreservesSensitiveHeadersSameHost is the control: a same-host +// redirect must keep behaving exactly as before the fix. +func TestRedirectPreservesSensitiveHeadersSameHost(t *testing.T) { + var finalSeen http.Header + + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/redirect": + w.Header().Set("Location", "http://target.test/dest") + w.WriteHeader(http.StatusFound) + case "/dest": + finalSeen = r.Header.Clone() + } + })) + defer target.Close() + + base := hostMappingTransport(t, map[string]string{ + "target.test": target.Listener.Addr().String(), + }) + + rt := &roundTripper{ + base: base, + cookie: "session=secret", + headers: map[string]string{"Authorization": "Bearer topsecret"}, + userAgent: "sif-scanner", + } + client := &http.Client{Transport: rt} + + resp, err := client.Get("http://target.test/redirect") + if err != nil { + t.Fatalf("get: %v", err) + } + DrainClose(resp) + + if finalSeen == nil { + t.Fatal("redirect never reached /dest") + } + if finalSeen.Get("Cookie") != "session=secret" { + t.Errorf("same-host Cookie = %q, want session=secret (same-host redirects keep it)", finalSeen.Get("Cookie")) + } + if finalSeen.Get("Authorization") != "Bearer topsecret" { + t.Errorf("same-host Authorization = %q, want Bearer topsecret (same-host redirects keep it)", finalSeen.Get("Authorization")) + } +}