Skip to content

Harden scanner against path traversal, NS takeover, and resource issues - #33

Merged
h0tak88r merged 6 commits into
masterfrom
testing
Jun 7, 2026
Merged

Harden scanner against path traversal, NS takeover, and resource issues#33
h0tak88r merged 6 commits into
masterfrom
testing

Conversation

@h0tak88r

@h0tak88r h0tak88r commented Jun 7, 2026

Copy link
Copy Markdown
Owner

No description provided.

h0tak88r and others added 3 commits June 7, 2026 19:56
…ce/race fixes

From an adversarial audit of internal/scanner/** (21 confirmed findings).
Fixes all HIGH + MEDIUM issues.

HIGH — path traversal (arbitrary file write + dir deletion):
Every scanner built its output dir as filepath.Join(resultsDir, <target>) where
the target (domain/subdomain/bucket/repo) was never stripped of '/' or '..'. Since
filepath.Join cleans the path, a target like "../../../../tmp/x" escaped the
results dir; the subdomain scan's end-of-run cleanup (RemoveAll) made it an
arbitrary-directory-deletion primitive, reachable from the unauthenticated-by-
default scan API.
- New utils.SanitizeTargetSegment(): collapses a target to one safe path segment
  (no separators, no ".."). Applied in utils.ResultsDir and at every direct
  path-build site: subdomain, nuclei, gf, sqlmap, dalfox, githubscan, backup,
  domain, ports, s3, asr, dns, misconfig, cnames, jsendpoints, urls, zerodays,
  exposure, depconfusion.
- Regression test asserts no sanitized segment can contain a separator/".." and
  cannot escape via filepath.Join.

HIGH — NS-takeover detection was dead code (dns.go):
runNSTakeover built its resolver with dnsx.DefaultOptions, which only queries A
records, so result.NS was always empty and NS takeovers were never detected.
Now configures QuestionTypes=[]uint16{dns.TypeNS} so the NS section is populated.

MEDIUM:
- jsscan: cap remote JS body with io.LimitReader(10MB) — unbounded io.ReadAll
  across ~100 workers could exhaust memory.
- cnames: on timeout, snapshot cnameRecords under the mutex before reading
  (workers could still append → data race).
- asr/dns_asr: puredns I/O used fixed /tmp filenames shared across runs; now uses
  a per-invocation os.MkdirTemp dir so concurrent ASR scans don't corrupt
  each other.

go mod tidy promotes miekg/dns (now used directly) and gjson to direct deps.

CI replicated locally: CGO_ENABLED=1 go vet/build/test all pass; new utils tests
and backup tests pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…r tests

The `*test*` pattern in .gitignore matched every Go `*_test.go` file, so the
test suite was silently untracked and CI's `go test ./...` never ran it (only
tracked files are checked out in CI). Add a `!*_test.go` negation so test files
are version-controlled again, and commit the regression test for
utils.SanitizeTargetSegment (the path-traversal guard added in the previous
commit).

Note: ~32 other pre-existing *_test.go files are now un-ignored and show as
untracked — they should be added in a follow-up so CI actually exercises them.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…aces)

From the scanner audit (LOW severity batch).

- ffuf try403Bypass (ffuf.go): closed response bodies per iteration instead of
  `defer` inside the bypass-technique loop (bodies/connections no longer
  accumulate until the function returns).
- bufio.Scanner 64KB limit (tempfiles.go, jsscan.go, urls.go, reflection.go,
  jsendpoints.go): raise the per-line buffer to 16MB so long Wayback/CDX URL
  lines no longer abort the read with ErrTooLong.
- cnames (cnames.go): call db.UpdateSubdomainCNAME synchronously inside the
  worker instead of a fire-and-forget `go` (was an unbounded goroutine leak that
  could outlive the function); use the derived root `domain`, not opts.Domain,
  so Targets/Subdomain modes record the correct key.
- githubscan (githubscan.go): run TruffleHog via exec.CommandContext with a
  30-minute timeout so a hung/huge org/repo can't pin the scan forever.
- subdomainmonitor (daemon.go): add an in-flight guard so a scan that outlasts
  its interval isn't started again by the next ticker pass (no more duplicate
  DB writes / duplicate webhook alerts).
- AEM (aem/scanner.go): close response bodies on every path. checkWebDAV never
  closed; ~20 probe loops closed only on HTTP 200, leaking every non-200 body.
  Added a per-request deferred close after each error check.

CI replicated locally: CGO_ENABLED=1 go vet/build/test all pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@gitguardian

gitguardian Bot commented Jun 7, 2026

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@h0tak88r

h0tak88r commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

Resolved in 00935ab. The 3 flagged ghp_… strings were synthetic test fixtures in internal/utils/pattern_matcher_test.go — fake inputs used to verify the secret-detection regex (ScanContentForSecrets), never real credentials. They're now constructed at runtime ("ghp_" + strings.Repeat(...)) so no token-shaped literal exists in source, and the test still exercises the same regex. Nothing to rotate. The GitGuardian incidents can be marked as false-positive / test fixtures in the dashboard.

h0tak88r and others added 2 commits June 7, 2026 22:00
The .gitignore *test* rule had silently excluded the Go test suite from version
control, so CI's go test ./... never ran it. Track the files (the .gitignore
negation was added in e0fdee4).

pattern_matcher_test.go builds its fake GitHub-PAT fixtures at runtime
("ghp_" + strings.Repeat(...)) so no token-shaped literal is ever committed —
they are synthetic inputs for the secret-detection regex, never real credentials.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previously the Programs page (program list + scope + cache) was HackerOne +
Bugcrowd only, and the Intigriti token env var was inconsistent.

- Direct Intigriti API client (programs_api.go): fetchITPrograms +
  fetchITScopeSummary against api.intigriti.com/external/researcher/v1. Built
  in-house (not bbscope) because bbscope calls log.Fatal on a bad token / HTTP
  error, which would os.Exit the whole server when the background warmer runs.
  Returns errors instead; concurrency-bounded scope enrichment with a
  rate-limit backoff.
- Wired Intigriti into apiListPrograms, serveProgramsPayload (platform filter),
  the cache warmer (buildProgramsPayload), and the cache payload (HasITToken).
- UI: Intigriti option in the platform filter, IT badge color/label, program
  count, and a "set INTIGRITI_TOKEN" hint when no token is configured.
- Env var: canonical INTIGRITI_TOKEN, with INTIGRITI_API_KEY accepted as an
  alias (intigritiToken()). Used by both the Programs page and the on-demand
  scope fetch (scope_api.go). Replaced the misspelled/dead INTEGRITI_API_KEY in
  config.go, env.example, and docker-compose, and documented the real platform
  credentials (H1_USERNAME/H1_TOKEN, BUGCROWD_TOKEN, INTIGRITI_TOKEN, YWH_TOKEN).

Caveat: the Intigriti API doesn't return per-target timestamps, so Intigriti
rows show scope counts + a representative target but no "updated X ago".

CI replicated locally: CGO_ENABLED=1 go vet/build/test pass; node --check on the
JS passes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@h0tak88r
h0tak88r merged commit f816a2d into master Jun 7, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant