From 5d00caa2b2f4036899274f9b1a869b703ca04772 Mon Sep 17 00:00:00 2001 From: Binay Date: Tue, 18 Aug 2026 15:53:58 -0400 Subject: [PATCH] fix(detection): a constant host with a tainted query string is not Go SSRF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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_core/pipeline/findings/lang_taint.py | 12 ++++++++++++ tests/test_findings_engine.py | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/signetry_core/pipeline/findings/lang_taint.py b/signetry_core/pipeline/findings/lang_taint.py index 4f8d3ba..826c4d4 100644 --- a/signetry_core/pipeline/findings/lang_taint.py +++ b/signetry_core/pipeline/findings/lang_taint.py @@ -38,6 +38,11 @@ class SinkSpec: remediation: str pattern: re.Pattern[str] requires_concat: bool = True # False for sinks where the value itself is the payload + # Optional negative guard: when this matches the sink line, the line is NOT a + # finding even though a tainted identifier appears on it. Used where taint on + # the line does not mean taint in the dangerous position (e.g. a constant URL + # host with a user-supplied query string is not SSRF). + skip_if: re.Pattern[str] | None = None confidence: float = 0.8 severity: Severity = Severity.HIGH @@ -96,6 +101,11 @@ def _idents(text: str, pat: re.Pattern[str]) -> set[str]: re.compile(r"\bhttp\.(?:Get|Head|Post|PostForm)\s*\(" r"|\b\w*[Cc]lient\.(?:Get|Head|Post|PostForm|Do)\s*\(" r"|\bhttp\.NewRequest(?:WithContext)?\s*\("), + # A literal that already covers scheme AND host pins the + # destination, so a tainted query string on the same line is not + # SSRF. Requiring >=1 host character before the closing quote or + # path keeps `"https://" + userHost` (real SSRF) firing. + skip_if=re.compile(r'"https?://[A-Za-z0-9.-]+[/"]'), requires_concat=False, confidence=0.8), SinkSpec("go.taint.path_traversal", "path_traversal", "CWE-22", "File path built from user input (taint)", @@ -268,6 +278,8 @@ def scan_lang_taint(file: str, text: str) -> list[Finding]: and _PARAM_PLACEHOLDER.search(line) and not spec.concat.search(line)): continue + if sink.skip_if is not None and sink.skip_if.search(line): + continue inline_src = bool(spec.source.search(line)) uses_tainted = any(v in tainted for v in _idents(line, spec.ident)) if not (inline_src or uses_tainted): diff --git a/tests/test_findings_engine.py b/tests/test_findings_engine.py index 8d1f749..afbd268 100644 --- a/tests/test_findings_engine.py +++ b/tests/test_findings_engine.py @@ -648,6 +648,25 @@ def test_lang_taint_go_ssrf(): assert "ssrf" in _cats(via_client, "proxy.go") +def test_lang_taint_go_ssrf_constant_host_is_not_ssrf(): + # A literal covering scheme AND host pins the destination, so a user-supplied + # query string or path suffix is not SSRF — the same distinction the Python rule + # is held to. Regression: this was a false positive against the eval corpus' + # LANG-60 safe decoy. + for src in ( + 'q := r.URL.Query().Get("q")\n' + 'resp, _ := http.Get("https://api.example.com/search?q=" + url.QueryEscape(q))\n', + 'id := r.URL.Query().Get("id")\n' + 'resp, _ := http.Get("https://api.example.com/items/" + id)\n', + ): + assert "ssrf" not in _cats(src, "client.go"), src + + # A literal that stops at the scheme does NOT pin the host — still SSRF. + attacker_host = ('h := r.URL.Query().Get("h")\n' + 'resp, _ := http.Get("https://" + h)\n') + assert "ssrf" in _cats(attacker_host, "client.go") + + def test_lang_taint_go_path_traversal(): src = ('name := r.URL.Query().Get("f")\n' 'data, _ := os.ReadFile("/var/data/" + name)\n')