diff --git a/internal/finding/finding.go b/internal/finding/finding.go index 5a4f739..e9f96bf 100644 --- a/internal/finding/finding.go +++ b/internal/finding/finding.go @@ -577,10 +577,17 @@ func flattenTakeover(target string, rs []scan.SubdomainTakeoverResult) []Finding if !t.Vulnerable { continue } + // a "potential" result only matched a body fingerprint and could not be + // checked against the cname (lookup unavailable), so it does not earn + // the same severity as a cname-confirmed takeover. + sev := sevTakeover + if t.Confidence == "potential" { + sev = SeverityMedium + } out = append(out, Finding{ Target: target, Module: "subdomain_takeover", - Severity: sevTakeover, + Severity: sev, Key: key("subdomain_takeover", t.Subdomain), Title: "takeover: " + t.Subdomain, Raw: t.Service, diff --git a/internal/scan/scan_test.go b/internal/scan/scan_test.go index 876e56e..889723a 100644 --- a/internal/scan/scan_test.go +++ b/internal/scan/scan_test.go @@ -20,7 +20,7 @@ func TestCheckSubdomainTakeover_GitHubPages(t *testing.T) { client := &http.Client{Timeout: 5 * time.Second} host := strings.TrimPrefix(server.URL, "http://") - vulnerable, service := checkSubdomainTakeover(host, client) + vulnerable, service, _ := checkSubdomainTakeover(host, client) if !vulnerable { t.Error("expected subdomain to be vulnerable") @@ -40,7 +40,7 @@ func TestCheckSubdomainTakeover_NotVulnerable(t *testing.T) { client := &http.Client{Timeout: 5 * time.Second} host := strings.TrimPrefix(server.URL, "http://") - vulnerable, service := checkSubdomainTakeover(host, client) + vulnerable, service, _ := checkSubdomainTakeover(host, client) if vulnerable { t.Error("expected subdomain to not be vulnerable") @@ -60,7 +60,7 @@ func TestCheckSubdomainTakeover_Heroku(t *testing.T) { client := &http.Client{Timeout: 5 * time.Second} host := strings.TrimPrefix(server.URL, "http://") - vulnerable, service := checkSubdomainTakeover(host, client) + vulnerable, service, _ := checkSubdomainTakeover(host, client) if !vulnerable { t.Error("expected subdomain to be vulnerable") @@ -80,7 +80,7 @@ func TestCheckSubdomainTakeover_AmazonS3(t *testing.T) { client := &http.Client{Timeout: 5 * time.Second} host := strings.TrimPrefix(server.URL, "http://") - vulnerable, service := checkSubdomainTakeover(host, client) + vulnerable, service, _ := checkSubdomainTakeover(host, client) if !vulnerable { t.Error("expected subdomain to be vulnerable") @@ -94,7 +94,7 @@ func TestCheckSubdomainTakeover_ConnectionError(t *testing.T) { client := &http.Client{Timeout: 1 * time.Second} // Use invalid host to simulate connection error - vulnerable, service := checkSubdomainTakeover("invalid.host.that.does.not.exist.local", client) + vulnerable, service, _ := checkSubdomainTakeover("invalid.host.that.does.not.exist.local", client) if vulnerable { t.Error("expected subdomain to not be vulnerable on connection error") diff --git a/internal/scan/subdomaintakeover.go b/internal/scan/subdomaintakeover.go index 884cc93..792c873 100644 --- a/internal/scan/subdomaintakeover.go +++ b/internal/scan/subdomaintakeover.go @@ -35,6 +35,10 @@ type SubdomainTakeoverResult struct { Subdomain string `json:"subdomain"` Vulnerable bool `json:"vulnerable"` Service string `json:"service,omitempty"` + // Confidence is "confirmed" when the cname chain resolves to the same + // provider as the matched signature, or "potential" when only the body + // fingerprint matched and the cname could not be checked against it. + Confidence string `json:"confidence,omitempty"` } // takeoverProviders maps a takeoverable third-party's cname apex to its service @@ -92,11 +96,12 @@ func SubdomainTakeover(url string, dnsResults []string, timeout time.Duration, t resultsChan := make(chan SubdomainTakeoverResult, len(dnsResults)) pool.Each(dnsResults, threads, func(subdomain string) { - vulnerable, service := checkSubdomainTakeover(subdomain, client) + vulnerable, service, confidence := checkSubdomainTakeover(subdomain, client) result := SubdomainTakeoverResult{ Subdomain: subdomain, Vulnerable: vulnerable, Service: service, + Confidence: confidence, } resultsChan <- result @@ -119,6 +124,26 @@ func SubdomainTakeover(url string, dnsResults []string, timeout time.Duration, t return results, nil } +// lookupCNAME resolves a subdomain's cname. it is a var so tests can stub out +// dns resolution instead of depending on a live resolver. +var lookupCNAME = func(ctx context.Context, host string) (string, error) { + return net.DefaultResolver.LookupCNAME(ctx, host) +} + +// serviceCorrelatable reports whether service has at least one cname apex in +// takeoverProviders, i.e. whether a cname lookup can ever confirm or contradict +// a body match for it. many body signatures (tumblr, intercom, and other +// custom-domain providers) have no apex entry, so a cname can neither back nor +// disprove them and must not be used to drop the finding. +func serviceCorrelatable(service string) bool { + for _, s := range takeoverProviders { + if s == service { + return true + } + } + return false +} + // danglingProvider reports whether cname points off-host at a known // takeoverable provider. a self-referential cname (LookupCNAME echoing an A // record back as the host) is rejected, since that's a live host, not a @@ -140,10 +165,10 @@ func danglingProvider(subdomain, cname string) (string, bool) { return "", false } -func checkSubdomainTakeover(subdomain string, client *http.Client) (bool, string) { +func checkSubdomainTakeover(subdomain string, client *http.Client) (bool, string, string) { req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, "http://"+subdomain, http.NoBody) if err != nil { - return false, "" + return false, "", "" } resp, err := client.Do(req) if err != nil { @@ -152,20 +177,20 @@ func checkSubdomainTakeover(subdomain string, client *http.Client) (bool, string // records, so "any cname" is not a signal - the cname must resolve to a // known takeoverable provider and not be the host itself. if strings.Contains(err.Error(), "no such host") { - cname, lookupErr := net.DefaultResolver.LookupCNAME(context.TODO(), subdomain) + cname, lookupErr := lookupCNAME(context.TODO(), subdomain) if lookupErr == nil { if service, ok := danglingProvider(subdomain, cname); ok { - return true, service + " (Dangling CNAME)" + return true, service + " (Dangling CNAME)", "confirmed" } } } - return false, "" + return false, "", "" } defer resp.Body.Close() body, err := io.ReadAll(io.LimitReader(resp.Body, 5*1024*1024)) if err != nil { - return false, "" + return false, "", "" } bodyString := string(body) @@ -201,10 +226,40 @@ func checkSubdomainTakeover(subdomain string, client *http.Client) (bool, string } for service, signature := range signatures { - if strings.Contains(bodyString, signature) { - return true, service + if !strings.Contains(bodyString, signature) { + continue } + + // a body fingerprint alone only proves the page renders a provider's + // unclaimed-domain copy - it says nothing about whether this host is + // actually delegated to that provider. a real github/s3/etc 404 page can + // show up on infrastructure with no dangling cname at all (a claimed page + // that happens to echo the same wording, or an unrelated host quoting + // it), so confirm the cname resolves to the SAME provider before calling + // it a takeover. a cname that resolves to something else (or to the host + // itself) contradicts the signature and must not be flagged. + cname, lookupErr := lookupCNAME(context.TODO(), subdomain) + if lookupErr == nil { + if cnameService, ok := danglingProvider(subdomain, cname); ok && cnameService == service { + return true, service, "confirmed" + } + // a resolving cname only contradicts the body match for providers we + // can actually correlate (an apex in takeoverProviders). rejecting + // those is the fp fix. for a provider with no apex to check against + // (tumblr and other custom-domain services), the cname proves + // nothing either way, so keep the body-only signal as low-confidence + // instead of silently dropping a real takeover. + if serviceCorrelatable(service) { + continue + } + return true, service, "potential" + } + + // cname lookup unavailable (resolver error, no records to compare): + // keep the body-only signal but mark it low-confidence rather than + // treating an unverified match as a confirmed takeover. + return true, service, "potential" } - return false, "" + return false, "", "" } diff --git a/internal/scan/subdomaintakeover_stale_test.go b/internal/scan/subdomaintakeover_stale_test.go index 8bab1d9..4fcbfec 100644 --- a/internal/scan/subdomaintakeover_stale_test.go +++ b/internal/scan/subdomaintakeover_stale_test.go @@ -35,7 +35,7 @@ func TestCheckSubdomainTakeover_CurrentUnclaimedPageDetected(t *testing.T) { _, _ = w.Write([]byte(c.body)) })) host := strings.TrimPrefix(srv.URL, "http://") - vulnerable, service := checkSubdomainTakeover(host, client) + vulnerable, service, _ := checkSubdomainTakeover(host, client) srv.Close() if !vulnerable || service != c.service { t.Errorf("%s unclaimed page not detected, got vulnerable=%v service=%q", c.service, vulnerable, service) @@ -57,7 +57,7 @@ func TestCheckSubdomainTakeover_StaleFingerprintRetired(t *testing.T) { _, _ = w.Write([]byte("" + body + "")) })) host := strings.TrimPrefix(srv.URL, "http://") - vulnerable, service := checkSubdomainTakeover(host, client) + vulnerable, service, _ := checkSubdomainTakeover(host, client) srv.Close() if vulnerable || service != "" { t.Errorf("stale fingerprint still raised a takeover (service %q) for body %q", service, body) diff --git a/internal/scan/subdomaintakeover_test.go b/internal/scan/subdomaintakeover_test.go index a929d63..f62352a 100644 --- a/internal/scan/subdomaintakeover_test.go +++ b/internal/scan/subdomaintakeover_test.go @@ -13,6 +13,7 @@ package scan import ( + "context" "net/http" "net/http/httptest" "strings" @@ -44,7 +45,7 @@ func TestCheckSubdomainTakeover_NotVulnerableServiceNotFlagged(t *testing.T) { } for service, fingerprint := range mitigated { subdomain := serveFingerprint(t, ""+fingerprint+"") - vulnerable, got := checkSubdomainTakeover(subdomain, client) + vulnerable, got, _ := checkSubdomainTakeover(subdomain, client) if vulnerable || got != "" { t.Errorf("%s fingerprint raised a takeover (vulnerable=%v service=%q); the provider mitigated the vector", service, vulnerable, got) } @@ -56,12 +57,74 @@ func TestCheckSubdomainTakeover_NotVulnerableServiceNotFlagged(t *testing.T) { func TestCheckSubdomainTakeover_VulnerableServiceStillFlagged(t *testing.T) { client := &http.Client{Timeout: 5 * time.Second} subdomain := serveFingerprint(t, "The specified bucket does not exist") - vulnerable, service := checkSubdomainTakeover(subdomain, client) + vulnerable, service, _ := checkSubdomainTakeover(subdomain, client) if !vulnerable || service != "Amazon S3" { t.Errorf("expected Amazon S3 takeover, got vulnerable=%v service=%q", vulnerable, service) } } +// reproduces the fixed false positive: a body fingerprint whose cname resolves +// elsewhere (or is a plain A record) must not be flagged. the pre-fix live path +// ignored the cname and fired on the body match alone. +func TestCheckSubdomainTakeover_BodySignatureWithoutMatchingCNAMENotConfirmed(t *testing.T) { + client := &http.Client{Timeout: 5 * time.Second} + subdomain := serveFingerprint(t, "The specified bucket does not exist") + + orig := lookupCNAME + defer func() { lookupCNAME = orig }() + + // cname resolves, but to something that isn't the s3 apex: the signature + // match is coincidental, not a dangling delegation. + lookupCNAME = func(_ context.Context, _ string) (string, error) { + return "unrelated-app.example.net.", nil + } + + vulnerable, service, confidence := checkSubdomainTakeover(subdomain, client) + if vulnerable { + t.Errorf("expected no takeover when cname does not back the matched provider, got vulnerable=%v service=%q confidence=%q", vulnerable, service, confidence) + } +} + +// when the cname does resolve to the matched provider's apex, the same body +// signature is a confirmed takeover, not just a potential one. +func TestCheckSubdomainTakeover_BodySignatureWithMatchingCNAMEConfirmed(t *testing.T) { + client := &http.Client{Timeout: 5 * time.Second} + subdomain := serveFingerprint(t, "The specified bucket does not exist") + + orig := lookupCNAME + defer func() { lookupCNAME = orig }() + + lookupCNAME = func(_ context.Context, _ string) (string, error) { + return "mybucket.s3.amazonaws.com.", nil + } + + vulnerable, service, confidence := checkSubdomainTakeover(subdomain, client) + if !vulnerable || service != "Amazon S3" || confidence != "confirmed" { + t.Errorf("expected confirmed Amazon S3 takeover, got vulnerable=%v service=%q confidence=%q", vulnerable, service, confidence) + } +} + +// a provider with no cname apex to correlate against (e.g. tumblr) must degrade +// to a low-confidence potential, not be dropped: dropping it would be a false +// negative on a real takeover. see serviceCorrelatable. +func TestCheckSubdomainTakeover_UncorrelatableProviderDegradesToPotential(t *testing.T) { + client := &http.Client{Timeout: 5 * time.Second} + subdomain := serveFingerprint(t, "There's nothing here.") + + orig := lookupCNAME + defer func() { lookupCNAME = orig }() + + // host resolves (lookup succeeds), but tumblr has no apex to check against. + lookupCNAME = func(_ context.Context, _ string) (string, error) { + return "domains.tumblr.com.", nil + } + + vulnerable, service, confidence := checkSubdomainTakeover(subdomain, client) + if !vulnerable || service != "Tumblr" || confidence != "potential" { + t.Errorf("expected potential Tumblr takeover, got vulnerable=%v service=%q confidence=%q", vulnerable, service, confidence) + } +} + // removed fingerprints matched generic content, not a provider's unclaimed-domain // page (none in can-i-take-over-xyz), so must not raise a takeover: activecampaign // was lighttpd's default page, kajabi/thinkific/tave/teamwork generic "not found". @@ -76,7 +139,7 @@ func TestCheckSubdomainTakeover_GenericFingerprintNotFlagged(t *testing.T) { } for name, body := range bogus { subdomain := serveFingerprint(t, body) - vulnerable, service := checkSubdomainTakeover(subdomain, client) + vulnerable, service, _ := checkSubdomainTakeover(subdomain, client) if vulnerable || service != "" { t.Errorf("%s raised a takeover (vulnerable=%v service=%q); it matches generic content, not an unclaimed-domain page", name, vulnerable, service) }