fix(scan): correlate cname before confirming subdomain takeover (#281)

the live-response path flagged any host whose body matched a provider
fingerprint without checking dns, so a page merely containing a
provider 404 string was reported as a takeover. resolve the cname and
only confirm when it maps to the same provider; a contradicting cname
kills the flag. providers with a body signature but no apex to
correlate, and unresolvable lookups, degrade to a medium potential
finding via a new confidence field rather than confirming or dropping.

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,

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")

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, "", ""
 }

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("<html><body>" + body + "</body></html>"))
 		}))
 		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)

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, "<html><body>"+fingerprint+"</body></html>")
-		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, "<html><body>The specified bucket does not exist</body></html>")
-	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, "<html><body>The specified bucket does not exist</body></html>")
+
+	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, "<html><body>The specified bucket does not exist</body></html>")
+
+	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, "<html><body>There's nothing here.</body></html>")
+
+	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)
 		}