fix(detection): a constant host with a tainted query string is not Go SSRF - #97
Merged
Conversation
… SSRF The go.taint.ssrf sink added in #95 fired whenever a tainted identifier appeared on the request line, which flagged this as SSRF: q := r.URL.Query().Get("q") resp, _ := http.Get("https://api.example.com/search?q=" + url.QueryEscape(q)) The destination host is compiled in; only the query string is user controlled. That is not SSRF — and it is the same false-positive class the Python SSRF rule is held to (constant URL with tainted params), so the Go rule should be held to it too. Found by the eval corpus rather than by inspection: the LANG-60 safe decoy in Signetry/eval scored 1 false positive while recall was 1.0. Adds an optional `skip_if` negative guard to SinkSpec — for sinks where a tainted identifier on the line does not imply taint in the dangerous position — and sets it on go.taint.ssrf as: "https?://[A-Za-z0-9.-]+[/"] Requiring at least one host character before the closing quote or path is what keeps the real attack firing: `http.Get("https://" + userHost)` has no host inside the literal, so the attacker still controls the destination and it is still reported. Verified both directions: tainted URL, `"https://" + host`, and a tainted value through client.Do still fire; constant host with a tainted query, a constant host with a tainted path suffix, and a fully constant URL do not.
Signetry Reviewer — 🟡 Needs human reviewA human should decide — the required check is pending. Deterministic gates (the authority)
FindingsNo issues found by the deterministic scanners. MergeA human should review and merge.
|
This was referenced Aug 18, 2026
bkd-dotcom
added a commit
to Signetry/eval
that referenced
this pull request
Aug 18, 2026
…os (#31) Closes #11, #12, #29, #13. Corpus goes 52 -> 60 cases: * LANG-53/54 XXE (CWE-611) in Java and PHP (#11) * LANG-56/57 path traversal (CWE-22) in Go and Java (#12) * LANG-59 SSRF (CWE-918) in Go (#29) * LANG-55/58/60 three SAFE decoys The decoys are the point as much as the positives: default-safe PHP XML parsing (entities are off by default on PHP 8+, so "parses XML" is not the bug), a constant filesystem path, and a constant host with a user-supplied query string. LANG-60 paid for itself immediately — it caught a false positive in signetry-core's brand-new go.taint.ssrf rule (constant host + tainted query string reported as SSRF), fixed in Signetry/core#97 before this landed. That is exactly the false-positive class the Python SSRF rule is held to, so the Go rule had to meet it too. Real repos (#13): OWASP WebGoat (Java) and OWASP RailsGoat (Ruby), both pinned to a full SHA. They are the first JVM/Ruby targets here — every existing case is Python or JavaScript, which under-exercises the multi-language tier. Fixes the pin mechanism while adding them. scan_real_repo cloned with --depth 1 then ran `git checkout <sha>` with check=False. On a shallow clone the object is absent, so the checkout failed with fatal: unable to read tree <sha> the failure was swallowed, and the scan silently ran against the default-branch tip — so a case documented as "pinned for reproducibility" was not pinned. Verified both behaviours against a real shallow clone. Now fetches the object first and, if pinning truly cannot be honoured, records that in the note instead of passing an unpinned scan off as pinned. Requires signetry-core v0.7.0 (pin bumped): the new cases depend on its Go SSRF / Go+Java path-traversal / PHP XXE rules. Verified against the published tag in a clean environment — 60 cases, recall 1.0, 0 false positives; 50 tests and ruff green. Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
This was referenced Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #95, found by the eval corpus rather than by inspection.
The false positive
go.taint.ssrffired whenever a tainted identifier appeared on the request line, so this was reported as SSRF:The destination host is compiled in — only the query string is user-controlled. That is not SSRF.
It is also exactly the false-positive class I held @AdvaitVarhade's Python SSRF PR to in #89 (constant URL with tainted
params/json/headers). The Go rule should meet the same bar.How it surfaced: while adding the eval corpus cases for Signetry/eval#11/#12/#29, the
LANG-60-SAFE-go-constant-urldecoy scored recall 1.0 but 1 false positive. The decoy was right and my rule was wrong.The fix
Adds an optional
skip_ifnegative guard toSinkSpec— for sinks where a tainted identifier on the line does not imply taint in the dangerous position. This mirrors the_PARAM_PLACEHOLDERspecial case the SQL sinks already have, but generically.On
go.taint.ssrf:The
[A-Za-z0-9.-]+is load-bearing. Requiring at least one host character before the closing quote or path is what keeps the real attack firing:http.Get(target)http.Get("https://" + userHost)"https://— no host charshttp.Get("https://api.example.com/search?q=" + q)/http.Get("https://api.example.com/items/" + id)/A naive
"https?://"guard would have silently broken the"https://" + userHostcase — a real SSRF where the attacker controls the whole host.Verification
278 tests pass,
ruffclean. New test asserts both directions. With this, the eval corpus scores recall 1.0 at 0 false positives across all 60 cases (verified locally against this branch).