mirror of
https://github.com/lunchcat/sif.git
synced 2026-07-28 14:37:01 -07:00
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.
This commit is contained in:
Tigah
+31
-4
@@ -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
|
||||
|
||||
@@ -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, `<a href="/go">follow me</a>`)
|
||||
})
|
||||
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, `<a href="/go">follow me</a>`)
|
||||
})
|
||||
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, `<a href="/p/%d">l</a>`, 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, `<a href="/secret">s</a>`)
|
||||
})
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user