From 788d20c18e9ca8449d7244f4d226266b88de21de Mon Sep 17 00:00:00 2001
From: Tigah <88289044+TBX3D@users.noreply.github.com>
Date: Wed, 22 Jul 2026 12:53:41 -0700
Subject: [PATCH] fix(crawl): stop redirects escaping crawl scope, cap fanout
(#333)
* docs(scan): correct crawl's false robots.txt claim
the doc comment said robots.txt is respected because colly honors it
by default, but colly's default is IgnoreRobotsTxt=true and Crawl never
flips it, so Disallow rules were never enforced. state the actual,
intentional behavior instead: a recon crawler has no reason to treat
Disallow as a scope boundary.
pin the behavior with a regression test so it isn't "fixed" into a
partial robots.txt implementation by accident later.
* fix(scan): cap crawl request fanout
MaxDepth only bounds recursion depth; colly's own MaxRequests budget
was left at 0 (unlimited) and Crawl set no Limit rule, so a hostile
page fanning out to an attacker-controlled number of links got every
one of them fetched at that depth (b^d). cap the total number of
requests a single Crawl call will make with a fixed internal budget.
TestCrawl_BoundsRequestFanout proves a 2500-link fanout page no longer
drives the server past maxCrawlRequests+1 actual fetches.
* fix(scan): stop redirects from escaping crawl scope
Crawl called SetClient(httpx.Client(timeout)), which replaces colly's
whole *http.Client and discards the CheckRedirect colly installs to
re-check AllowedDomains on every redirect hop. with it gone, redirects
fell back to net/http's default: follow up to 10 hops to any host. an
in-scope page could 302 the crawler to an arbitrary off-scope host
(e.g. a cloud metadata endpoint) and it would be fetched.
colly's own AllowedDomains only matches on hostname anyway, which
would still let a same-host, different-port redirect through (an
internal service reachable on the same IP). build our own http.Client
on top of httpx's shared transport instead, with a CheckRedirect that
requires the redirect target to share the exact host:port of the
request that produced it, and caps the hop count ourselves since a
custom CheckRedirect drops net/http's own 10-redirect default.
TestCrawl_RedirectRejectsOffScopeHost proves the off-scope host is no
longer fetched; TestCrawl_RedirectFollowsSameHost proves ordinary
same-host redirects still are.
---
internal/scan/crawl.go | 35 ++++++++--
internal/scan/crawl_test.go | 132 ++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+), 4 deletions(-)
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)
+ }
+}