diff --git a/internal/scan/crawl.go b/internal/scan/crawl.go index 55f908e..4ffdac3 100644 --- a/internal/scan/crawl.go +++ b/internal/scan/crawl.go @@ -14,6 +14,7 @@ package scan import ( "fmt" + "net/http" "net/url" "sort" "sync" @@ -43,9 +44,19 @@ func (r *CrawlResult) ResultType() string { return "crawl" } // compile-time check so a result-type drift fails the build, not a run. var _ ScanResult = (*CrawlResult)(nil) +// maxCrawlRequests caps the total pages a single Crawl call will fetch, so a +// hostile page fanning out to an attacker-controlled number of links can't +// turn a bounded-depth crawl into an unbounded one (b^d). +const maxCrawlRequests = 2000 + +// maxRedirectHops mirrors net/http's own default redirect cap, which we lose +// once we install a custom CheckRedirect below. +const maxRedirectHops = 10 + // Crawl spiders the target up to depth, following same-host links/scripts/forms. // all traffic flows through the shared httpx client so proxy/headers/rate-limit -// apply, and robots.txt is respected (colly honors it by default). +// apply. robots.txt is intentionally NOT honored: this is a recon/pentest +// crawler and Disallow rules are not a scope boundary we want to respect. func Crawl(targetURL string, depth int, timeout time.Duration, logdir string) (*CrawlResult, error) { log := output.Module("CRAWL") log.Start() @@ -72,10 +83,26 @@ func Crawl(targetURL string, depth int, timeout time.Duration, logdir string) (* collector := colly.NewCollector( colly.MaxDepth(depth), colly.AllowedDomains(host), + colly.MaxRequests(maxCrawlRequests), ) - // reuse the shared client so proxy/cookie/-H/rate-limit are honored and the - // configured timeout applies to every fetch, robots.txt included. - collector.SetClient(httpx.Client(timeout)) + // reuse the shared transport so proxy/-H/rate-limit still apply, but scope + // redirects ourselves: colly's CheckRedirect only re-checks AllowedDomains, + // which matches on hostname alone, so a same-host redirect to a different + // port would still be followed. also re-cap the hop count, since installing + // a custom CheckRedirect drops net/http's own 10-redirect default. + collector.SetClient(&http.Client{ + Timeout: timeout, + Transport: httpx.Client(timeout).Transport, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirectHops { + return fmt.Errorf("stopped after %d redirects", maxRedirectHops) + } + if req.URL.Host != via[0].URL.Host { + return fmt.Errorf("redirect to %q leaves crawl scope %q", req.URL, via[0].URL.Host) + } + return nil + }, + }) // dedupe across the concurrent callbacks colly may fire. var mu sync.Mutex diff --git a/internal/scan/crawl_test.go b/internal/scan/crawl_test.go index c0cc260..770d07d 100644 --- a/internal/scan/crawl_test.go +++ b/internal/scan/crawl_test.go @@ -13,8 +13,10 @@ package scan import ( + "fmt" "net/http" "net/http/httptest" + "sync/atomic" "testing" "time" ) @@ -156,3 +158,133 @@ func TestCrawl_ResultType(t *testing.T) { t.Errorf("ResultType = %q, want crawl", r.ResultType()) } } + +// an in-scope page 302-redirecting to a separate host must not be followed: +// colly's own CheckRedirect only re-checks AllowedDomains, which matches on +// hostname alone, so a same-host redirect to a different port (as an +// internal service or metadata endpoint reachable on the same host would be) +// slipped through until Crawl installed its own host:port-strict check. +func TestCrawl_RedirectRejectsOffScopeHost(t *testing.T) { + var offScopeHits int64 + offScope := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt64(&offScopeHits, 1) + _, _ = w.Write([]byte("should never be fetched")) + })) + defer offScope.Close() + + mux := http.NewServeMux() + mux.HandleFunc("/robots.txt", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/go" { + http.Redirect(w, r, offScope.URL+"/secret", http.StatusFound) + return + } + _, _ = fmt.Fprint(w, `follow me`) + }) + target := httptest.NewServer(mux) + defer target.Close() + + if _, err := Crawl(target.URL, 3, 5*time.Second, ""); err != nil { + t.Fatalf("Crawl: %v", err) + } + + if got := atomic.LoadInt64(&offScopeHits); got != 0 { + t.Errorf("redirect escaped scope: off-scope host fetched %d time(s)", got) + } +} + +// a redirect that stays on the same host:port must still be followed; the +// scope check must not break ordinary same-site redirects (e.g. login +// bounces, trailing-slash normalization). +func TestCrawl_RedirectFollowsSameHost(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/robots.txt", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/go", func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/landed", http.StatusFound) + }) + mux.HandleFunc("/landed", func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`leaf`)) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + _, _ = fmt.Fprint(w, `follow me`) + }) + target := httptest.NewServer(mux) + defer target.Close() + + result, err := Crawl(target.URL, 3, 5*time.Second, "") + if err != nil { + t.Fatalf("Crawl: %v", err) + } + + if !urlsContain(result.URLs, target.URL+"/go") { + t.Errorf("expected same-host redirect source /go to be recorded, got %v", result.URLs) + } +} + +// a hostile page fanning out to far more links than any real page would must +// not turn a bounded-depth crawl into an unbounded one. child pages are +// leaves with no links of their own, so this exercises maxCrawlRequests +// rather than the (unbounded, by design) link count off a single page. +func TestCrawl_BoundsRequestFanout(t *testing.T) { + fanout := maxCrawlRequests + 500 + var hits int64 + + mux := http.NewServeMux() + mux.HandleFunc("/robots.txt", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(&hits, 1) + if r.URL.Path != "/" { + _, _ = w.Write([]byte("leaf")) + return + } + for i := 0; i < fanout; i++ { + _, _ = fmt.Fprintf(w, `l`, i) + } + }) + target := httptest.NewServer(mux) + defer target.Close() + + if _, err := Crawl(target.URL, 1, 30*time.Second, ""); err != nil { + t.Fatalf("Crawl: %v", err) + } + + // +1 for the root page itself; the rest must be capped at maxCrawlRequests. + if got, want := atomic.LoadInt64(&hits), int64(maxCrawlRequests+1); got > want { + t.Errorf("expected at most %d requests (request cap), server received %d", want, got) + } +} + +// robots.txt is intentionally NOT honored: sif is a recon/pentest crawler and +// Disallow rules are not a scope boundary it should respect. This pins the +// intentional behavior so it isn't "fixed" into a partial robots.txt +// implementation by accident later. +func TestCrawl_DoesNotHonorRobots(t *testing.T) { + var secretHits int64 + mux := http.NewServeMux() + mux.HandleFunc("/robots.txt", func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("User-agent: *\nDisallow: /\n")) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/secret" { + atomic.AddInt64(&secretHits, 1) + _, _ = w.Write([]byte("leaf")) + return + } + _, _ = fmt.Fprint(w, `s`) + }) + target := httptest.NewServer(mux) + defer target.Close() + + if _, err := Crawl(target.URL, 2, 5*time.Second, ""); err != nil { + t.Fatalf("Crawl: %v", err) + } + if got := atomic.LoadInt64(&secretHits); got == 0 { + t.Errorf("expected Disallow:/ path to be fetched (robots.txt is not honored), got %d hits", got) + } +}