From 12067b4279f1d3d07d5cbb960245d2b16697a9fd Mon Sep 17 00:00:00 2001 From: Tigah <88289044+TBX3D@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:48:40 -0700 Subject: [PATCH] fix(scan): gate cors http-downgrade probe on https targets only (#305) the http scheme downgrade origin was queued against every target, so on a plain-http target the http://host probe is the target's own origin and any correct same-origin reflection was reported as a high-severity downgrade. mark that origin https-only and skip it when the target scheme is not https, where a downgrade is meaningless, while still firing it on https targets that genuinely trust the http origin. --- internal/scan/cors.go | 28 ++++++++++----- internal/scan/cors_test.go | 71 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/internal/scan/cors.go b/internal/scan/cors.go index 7b11ec6..d7d6ed4 100644 --- a/internal/scan/cors.go +++ b/internal/scan/cors.go @@ -52,19 +52,25 @@ var corsOrigins = []struct { origin string // crafted Origin header, {host} -> target host note string // why this case is interesting reflects bool // true when a literal echo of this origin is exploitable + // httpsOnly is set for origins that are only a genuine bypass when the + // target itself is https; against a plain http target the crafted origin + // is the target's own real origin, so reflecting it isn't a bug. + httpsOnly bool }{ // arbitrary attacker origin - the classic "reflects anything" bug - {corsEvilOrigin, "arbitrary origin reflected", true}, + {corsEvilOrigin, "arbitrary origin reflected", true, false}, // the literal null origin (sandboxed iframes, redirects, file://) is forgeable - {"null", "null origin allowed", true}, + {"null", "null origin allowed", true, false}, // suffix bypass: attacker registers {host}.evil.com, naive endswith checks pass - {"https://{host}.evil.com", "suffix bypass (attacker subdomain)", true}, + {"https://{host}.evil.com", "suffix bypass (attacker subdomain)", true, false}, // prefix bypass: attacker registers evil-{host}, naive startswith checks pass - {"https://evil-{host}", "prefix bypass", true}, + {"https://evil-{host}", "prefix bypass", true, false}, // embedded bypass: {host} appears inside an attacker domain - {"https://evil.com.{host}", "embedded-host bypass", true}, - // scheme downgrade: http origin trusted lets a mitm read cross-origin data - {"http://{host}", "http scheme downgrade trusted", true}, + {"https://evil.com.{host}", "embedded-host bypass", true, false}, + // scheme downgrade: http origin trusted lets a mitm read cross-origin data. + // only a real downgrade if the target itself is https; against a plain + // http target this origin equals the target's own origin. + {"http://{host}", "http scheme downgrade trusted", true, true}, } // CORS probes the target for cross-origin resource sharing misconfigurations. @@ -91,6 +97,7 @@ func CORS(targetURL string, timeout time.Duration, threads int, logdir string) ( return nil, fmt.Errorf("parse url: %w", err) } host := parsedURL.Host + isHTTPS := parsedURL.Scheme == "https" client := httpx.Client(timeout) // don't follow redirects: cors is judged on the host we asked about, so a @@ -104,9 +111,14 @@ func CORS(targetURL string, timeout time.Duration, threads int, logdir string) ( var mu sync.Mutex var wg sync.WaitGroup - // one origin per worker item; the set is small so a buffered channel is plenty + // one origin per worker item; the set is small so a buffered channel is plenty. + // skip httpsOnly origins against a non-https target: the crafted origin + // would just be the target's own real origin, not a bypass. originChan := make(chan int, len(corsOrigins)) for i := 0; i < len(corsOrigins); i++ { + if corsOrigins[i].httpsOnly && !isHTTPS { + continue + } originChan <- i } close(originChan) diff --git a/internal/scan/cors_test.go b/internal/scan/cors_test.go index 678613c..30dd107 100644 --- a/internal/scan/cors_test.go +++ b/internal/scan/cors_test.go @@ -13,6 +13,7 @@ package scan import ( + "crypto/tls" "net/http" "net/http/httptest" "sync/atomic" @@ -165,6 +166,76 @@ func TestCORS_JudgesRequestedHostNotRedirectTarget(t *testing.T) { } } +// TestCORS_NoDowngradeFindingOnPlainHTTPTarget pins the scheme-downgrade probe: +// httptest.NewServer targets are plain http, so an "http://{host}" origin is +// not a downgrade at all, it is the target's own real origin. reflecting it +// back is normal same-origin behavior, not a misconfiguration. +func TestCORS_NoDowngradeFindingOnPlainHTTPTarget(t *testing.T) { + srv := reflectingCORS() + defer srv.Close() + + result, err := CORS(srv.URL, 5*time.Second, 3, "") + if err != nil { + t.Fatalf("CORS: %v", err) + } + if result == nil { + return + } + for _, f := range result.Findings { + if f.Note == "http scheme downgrade trusted" { + t.Errorf("expected no downgrade finding against a plain http target, got %+v", f) + } + } +} + +// TestCORS_DowngradeFiresOnHTTPSTarget is the counterpart to the plain-http +// case: against an https target that reflects an http origin with credentials, +// an on-path attacker can read authenticated data, so the downgrade probe must +// still fire high. this pins the gate to the target scheme, not the origin +// scheme, so the false-positive fix cannot silently become a false negative. +func TestCORS_DowngradeFiresOnHTTPSTarget(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if origin := r.Header.Get("Origin"); origin != "" { + w.Header().Set("Access-Control-Allow-Origin", origin) + w.Header().Set("Access-Control-Allow-Credentials", "true") + } + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + // CORS builds its client from the unconfigured httpx transport, which is + // http.DefaultTransport; trust the self-signed test cert for this test only. + orig := http.DefaultTransport + tr := http.DefaultTransport.(*http.Transport).Clone() + tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint:gosec // trusts the local test cert only + http.DefaultTransport = tr + defer func() { http.DefaultTransport = orig }() + + result, err := CORS(srv.URL, 5*time.Second, 3, "") + if err != nil { + t.Fatalf("CORS: %v", err) + } + if result == nil { + t.Fatalf("expected findings on https reflecting server, got nil") + } + var sawDowngrade bool + for _, f := range result.Findings { + if f.Note != "http scheme downgrade trusted" { + continue + } + sawDowngrade = true + if f.Severity != "high" { + t.Errorf("expected high severity for https-trusts-http with creds, got %s", f.Severity) + } + if !f.AllowCredentials { + t.Errorf("expected credentials flagged on downgrade finding, got %+v", f) + } + } + if !sawDowngrade { + t.Errorf("expected a downgrade finding against an https target, got %+v", result.Findings) + } +} + func TestCORSResult_ResultType(t *testing.T) { r := &CORSResult{} if r.ResultType() != "cors" {