From d87ed9212926cdf4d4c32710b750b2ba9e4cecf3 Mon Sep 17 00:00:00 2001 From: Kunal Jaura Date: Mon, 14 Sep 2026 10:13:45 -0700 Subject: [PATCH] feat(go): wrapped-handler routes, XSS rule, and govwa SAST benchmark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gaps a benchmark against a real vulnerable Go app (govwa) surfaced. All changes are Go-only (route mapper + Go taint pack + benchmark), so no other language is affected. Route mapper — middleware-wrapped handlers: - `router.GET(p, Log(RequireAuth(h)))` is now recognized as a route (balanced- paren arg capture), where before only bare-name/inline handlers were — so idiomatic Go middleware chaining no longer yields zero routes. - Auth is credited only from a wrapping middleware CALL (name followed by `(`), never from a handler merely NAMED with an auth-ish word — marking a route authed off a handler name would be the dangerous direction (suppresses a real missing-auth). Innermost resolvable handler supplies handler_source. Go XSS rule (injection-go.yaml): - Flags user data cast to a template safe-string type (template.HTML/JS/ HTMLAttr/CSS/URL), which bypasses html/template auto-escaping (gosec G203). Implemented as pattern-regex, not an AST pattern, because the cast is a Go type conversion whose AST match needs type resolution that a cloned repo (no downloaded modules) lacks. A string-literal cast is excluded — no FP on static markup. go-sast benchmark target: - benchmarks/go_sast.py clones govwa at a pinned commit and scores CLASS-LEVEL recall (SQLi, XSS, vulnerable-dependency, missing-auth) — deliberately not pinned to line numbers or app symbols, so the engine stays app-agnostic. Opt-in (named or --all, never the default set); skips cleanly when git/network or go/gopls are unavailable. Wired into run_benchmarks.py. Verified: - govwa full scan: 7 -> 16 findings; routes 0 -> 20; missing-auth 0 -> 5; auth-verified 15/20; all four vuln classes caught. go-sast: 4/4 class recall. - SAST-injection fixture gate extended with a Go XSS case: 53/53, 0 FP. - Full suite: 2621 passed, 1 xfailed. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EZ4QoTqRoYWE25CNoV2Wfy --- benchmarks/README.md | 11 +- .../sast-injection/safe/SafeHandler.go | 7 + .../vulnerable/InjectionHandler.go | 8 + benchmarks/go_sast.py | 162 ++++++++++++++++++ benchmarks/run_benchmarks.py | 64 +++++-- benchmarks/sast_injection.py | 3 +- .../engine/code_analysis/go_route_mapper.py | 109 +++++++++--- .../semgrep_rules/injection-go.yaml | 19 ++ .../benchmarks/test_sast_injection_scorer.py | 64 ++++++- .../test_go_route_mapper.py | 48 ++++++ 10 files changed, 446 insertions(+), 49 deletions(-) create mode 100644 benchmarks/go_sast.py diff --git a/benchmarks/README.md b/benchmarks/README.md index 708c0e0..da83fa7 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -14,12 +14,16 @@ python benchmarks/run_benchmarks.py juiceshop-writes # + --probe-writes (mutate python benchmarks/run_benchmarks.py --all # + NodeGoat + crAPI + Juice Shop (heavy) python benchmarks/run_benchmarks.py crapi # a single target python benchmarks/run_benchmarks.py sast-injection # taint recall/FP, no Docker +python benchmarks/run_benchmarks.py go-sast # Go recall vs real govwa (clones a pinned commit) python benchmarks/run_benchmarks.py --keep vampi-vulnerable # leave it running python benchmarks/run_benchmarks.py juiceshop --report-dir /tmp/r # keep the raw JSON reports ``` -Docker is required for every target **except** `sast-injection`, which instead -needs the `semgrep` binary (`pipx install semgrep` or `pip install semgrep`). +Docker is required for every target **except** `sast-injection` and `go-sast`, +which instead need the `semgrep` binary (`pipx install semgrep` or +`pip install semgrep`). `go-sast` also clones the real `govwa` app at a pinned +commit (needs `git`; `go`+`gopls` add the route/auth half) and is opt-in — named +explicitly or via `--all`, never in the default run. `--report-dir DIR` keeps each target's raw JSON report instead of discarding it. The scorecard says *how many* findings went unmatched but not *which*, and @@ -54,7 +58,8 @@ For each target the scorecard reports two things — both matter: | `crapi` | microservices | upstream compose (auto-cloned) | OWASP API Top 10; IDOR/BAC/auth depth | | `juiceshop` | Angular/Express SPA | single image | **headline recall** — scored per-challenge vs the app's own `/api/Challenges` | | `juiceshop-auth` | same, authenticated | single image | adds the login-gated challenges (the `~40%` number) | -| `sast-injection` | JS/TS fixtures | **none** (code-only) | **taint layer recall/FP** — deterministic SAST injection floor (#4) | +| `sast-injection` | JS/TS/Python/Java/Kotlin/Go fixtures | **none** (code-only) | **taint layer recall/FP** — deterministic SAST injection floor (#4) | +| `go-sast` | real govwa app (pinned clone) | **none** (code-only) | **Go recall** — SQLi/XSS/dep/missing-auth class recall across the whole Go SAST path (opt-in; needs `git`, and `go`/`gopls` for the auth half) | VAmPI is a single container and runs in the default set. NodeGoat and crAPI are heavier (compose, mongo, several GB for crAPI) — run them individually. They are diff --git a/benchmarks/fixtures/sast-injection/safe/SafeHandler.go b/benchmarks/fixtures/sast-injection/safe/SafeHandler.go index 488e0ba..10ac248 100644 --- a/benchmarks/fixtures/sast-injection/safe/SafeHandler.go +++ b/benchmarks/fixtures/sast-injection/safe/SafeHandler.go @@ -4,6 +4,7 @@ package safe import ( "database/sql" + "html/template" "net/http" "os" "os/exec" @@ -31,3 +32,9 @@ func Download(w http.ResponseWriter, r *http.Request) { f, _ := os.Open(filepath.Join("/data", name)) _ = f } + +// Static markup cast to template.HTML is the intentional, safe use — a string +// literal, not user input, so it must NOT be flagged as XSS. +func Banner() template.HTML { + return template.HTML("Welcome") +} diff --git a/benchmarks/fixtures/sast-injection/vulnerable/InjectionHandler.go b/benchmarks/fixtures/sast-injection/vulnerable/InjectionHandler.go index 9e47763..3e50745 100644 --- a/benchmarks/fixtures/sast-injection/vulnerable/InjectionHandler.go +++ b/benchmarks/fixtures/sast-injection/vulnerable/InjectionHandler.go @@ -6,6 +6,7 @@ package vulnerable import ( "database/sql" "fmt" + "html/template" "net/http" "os" "os/exec" @@ -51,3 +52,10 @@ func Download(w http.ResponseWriter, r *http.Request) { f, _ := os.Open("/data/" + name) // EXPECT path-traversal _ = f } + +// --- XSS: casting user input to template.HTML bypasses auto-escaping --- +func Greet(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + data := map[string]interface{}{"name": template.HTML(name)} // EXPECT reflected-xss + _ = data +} diff --git a/benchmarks/go_sast.py b/benchmarks/go_sast.py new file mode 100644 index 0000000..40e789b --- /dev/null +++ b/benchmarks/go_sast.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +"""Go SAST benchmark — recall against a real vulnerable Go app (govwa). + +Unlike ``sast_injection.py`` (a curated fixture tree with per-line ``// EXPECT`` +markers), this scores a code-only scan of a *real* intentionally-vulnerable app — +`govwa `_ (Go Vulnerable Web Application) — pinned +to a commit so the ground truth doesn't drift. It exercises the whole Go SAST +path together: the injection taint pack, the route mapper (middleware-wrapped +handlers), route/auth analysis, and OSV dependency scanning. + +Ground truth is CLASS-LEVEL, not per-line: govwa is documented to contain SQLi, +XSS, IDOR, and it ships an old gin (CVEs) plus unauthenticated setup routes. We +assert the scan surfaces each class at least once — deliberately NOT pinned to +line numbers or govwa-internal symbols, so the engine stays app-agnostic and the +benchmark survives cosmetic upstream edits. + +Usage: + python benchmarks/go_sast.py # clone (pinned) + scan + score + python benchmarks/go_sast.py findings.json # score an existing scan JSON + +Requires (scan path): ``isitsecure`` + the ``semgrep`` binary, ``git``, and — for +the route/auth half — the ``go`` toolchain + ``gopls`` on PATH. Missing tools +SKIP (exit 0) rather than fail, so CI without them is not blocked. +""" + +from __future__ import annotations + +import json +import pathlib +import shutil +import subprocess +import sys +import tempfile + +REPO_URL = "https://github.com/0c34/govwa" +# Pinned so ground truth can't drift (2025-02-20). +PIN = "4058f79f31eeb4a36d8f1e64bba1f0c899646e6f" +CLONE_DIR = pathlib.Path(__file__).parent / "_ext" / "govwa" + +# Class-level ground truth: (label, predicate over a finding dict). +# Each MUST be satisfied by at least one finding. Kept to category + broad +# keywords so it checks capability, not exact wording. +EXPECTED = { + "sqli": lambda f: _is(f, "injection") and _kw(f, "sql"), + "xss": lambda f: _is(f, "injection") and _kw(f, "xss", "cross-site", "template.html"), + "vulnerable-dependency": lambda f: _cat(f) in ("dependency_vuln", "vulnerable_dependency") + or _kw(f, "vulnerable dependency"), + "missing-auth": lambda f: _cat(f) == "auth_weakness" and _kw(f, "auth"), +} + + +def _cat(f: dict) -> str: + c = f.get("category", "") + return c.get("value", "") if isinstance(c, dict) else str(c) + + +def _text(f: dict) -> str: + return f"{f.get('title', '')} {f.get('description', '')}".lower() + + +def _kw(f: dict, *words: str) -> bool: + t = _text(f) + return any(w in t for w in words) + + +def _is(f: dict, cat_substr: str) -> bool: + return cat_substr in _cat(f).lower() + + +def score(findings: list[dict]) -> dict: + by_class = {} + for label, pred in EXPECTED.items(): + hits = [f for f in findings if pred(f)] + by_class[label] = {"found": len(hits), "detected": bool(hits)} + found = sum(1 for v in by_class.values() if v["detected"]) + return { + "by_class": by_class, + "recall": {"found": found, "total": len(EXPECTED)}, + "total_findings": len(findings), + } + + +def passed(result: dict) -> bool: + return result["recall"]["found"] == result["recall"]["total"] + + +def _have(*bins: str) -> bool: + return all(shutil.which(b) for b in bins) + + +def ensure_clone() -> bool: + """Clone govwa at the pinned commit. Returns False if git/network is + unavailable (the benchmark then SKIPs rather than fails).""" + if (CLONE_DIR / "go.mod").exists(): + return True + if not shutil.which("git"): + return False + CLONE_DIR.parent.mkdir(parents=True, exist_ok=True) + try: + subprocess.run(["git", "clone", REPO_URL, str(CLONE_DIR)], # noqa: S603,S607 + check=True, timeout=180, capture_output=True) + subprocess.run(["git", "-C", str(CLONE_DIR), "checkout", PIN], # noqa: S603,S607 + check=True, timeout=60, capture_output=True) + return True + except Exception as exc: # noqa: BLE001 + print(f" clone failed ({exc}) — skipping") + shutil.rmtree(CLONE_DIR, ignore_errors=True) + return False + + +def run_scan() -> list[dict]: + out = tempfile.NamedTemporaryFile("r", suffix=".json", delete=False).name + cmd = ["isitsecure", "scan", "-r", f"file://{CLONE_DIR.resolve()}", + "--mode", "code-only", "--llm", "none", "--output", "json", "-f", out] + print(f"$ {' '.join(cmd)}") + try: + subprocess.run(cmd, check=True, timeout=600) # noqa: S603 + data = json.loads(pathlib.Path(out).read_text()) + return data.get("findings", []) if isinstance(data, dict) else data + finally: + pathlib.Path(out).unlink(missing_ok=True) + + +def print_report(r: dict) -> None: + rec = r["recall"] + pct = (100 * rec["found"] / rec["total"]) if rec["total"] else 0 + print("=" * 60) + print("GO SAST BENCHMARK — govwa class-level recall") + print("=" * 60) + print(f"\nClass recall: {rec['found']}/{rec['total']} ({pct:.0f}%)") + print(f"Total findings: {r['total_findings']}") + print("\nBy class (detected? / count):") + for cls, v in r["by_class"].items(): + mark = "x" if v["detected"] else " " + print(f" [{mark}] {cls:22} {v['found']}") + + +def main() -> int: + if len(sys.argv) == 2: + data = json.loads(pathlib.Path(sys.argv[1]).read_text()) + findings = data.get("findings", data) if isinstance(data, dict) else data + else: + if not _have("isitsecure", "semgrep"): + print("SKIPPED — need `isitsecure` and `semgrep` on PATH.") + return 0 + if not _have("go", "gopls"): + print("NOTE — `go`/`gopls` not on PATH; route/auth half is degraded " + "(injection + dependency recall still scored).") + if not ensure_clone(): + print("SKIPPED — could not obtain govwa (git/network unavailable).") + return 0 + findings = run_scan() + result = score(findings) + print_report(result) + ok = passed(result) + print(f"\n{'PASS' if ok else 'FAIL'} — class recall " + f"{result['recall']['found']}/{result['recall']['total']}") + return 0 if ok else 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/benchmarks/run_benchmarks.py b/benchmarks/run_benchmarks.py index 716c49c..e729747 100644 --- a/benchmarks/run_benchmarks.py +++ b/benchmarks/run_benchmarks.py @@ -495,25 +495,34 @@ def print_scorecard(results: list[dict]) -> None: # code-only scan of a local fixture tree (see sast_injection.py). It's exposed # here as a pseudo-target so it runs from the same entrypoint (and by default). SAST_INJECTION = "sast-injection" +# Go SAST recall against the real govwa app. Opt-in only (named or --all): it +# clones an external repo and wants the go/gopls toolchain, so it must not run +# in the default no-arg set (which stays offline-friendly). +GO_SAST = "go-sast" +_PSEUDO = {SAST_INJECTION, GO_SAST} -def resolve_selection(targets: list[str], all_flag: bool) -> tuple[list[str], bool, list[str]]: +def resolve_selection( + targets: list[str], all_flag: bool +) -> tuple[list[str], bool, bool, list[str]]: """Plan a run from the CLI args (pure — no Docker, no side effects). - Returns (docker target names to run, whether to run sast-injection, unknown names). - The SAST pseudo-target runs when named, with --all, or in the default (no-arg) set. + Returns (docker names, run sast-injection?, run go-sast?, unknown names). + sast-injection runs when named, with --all, or in the default (no-arg) set. + go-sast runs only when named or with --all (never in the default set). """ valid = {t.name for t in TARGETS} want_sast = SAST_INJECTION in targets or all_flag or not targets - docker_names = [n for n in targets if n != SAST_INJECTION] + want_go_sast = GO_SAST in targets or all_flag + docker_names = [n for n in targets if n not in _PSEUDO] if docker_names: unknown = [n for n in docker_names if n not in valid] - return [n for n in docker_names if n in valid], want_sast, unknown + return [n for n in docker_names if n in valid], want_sast, want_go_sast, unknown if all_flag: - return [t.name for t in TARGETS], want_sast, [] - if targets: # only sast-injection was requested - return [], want_sast, [] - return ["vampi-vulnerable", "vampi-secure"], want_sast, [] + return [t.name for t in TARGETS], want_sast, want_go_sast, [] + if targets: # only pseudo-targets were requested + return [], want_sast, want_go_sast, [] + return ["vampi-vulnerable", "vampi-secure"], want_sast, want_go_sast, [] def run_sast_injection() -> int: @@ -539,6 +548,32 @@ def run_sast_injection() -> int: return 0 if si.passed(r) else 1 +def run_go_sast() -> int: + """Run the Go SAST benchmark (govwa); 0 on full class recall, else 1. + + Skips (returns 0) when its tools or the network are unavailable — like the + Docker targets skip without Docker. + """ + import go_sast as gs + + print(f"\n=== {GO_SAST} ===\n code-only recall against the real govwa " + f"app (clones a pinned commit; needs semgrep, and go/gopls for auth)") + if not gs._have("isitsecure", "semgrep"): + print(" SKIPPED — need isitsecure + semgrep on PATH.") + return 0 + if not gs.ensure_clone(): + print(" SKIPPED — could not obtain govwa (git/network unavailable).") + return 0 + try: + findings = gs.run_scan() + except Exception as e: # noqa: BLE001 - surface, don't crash the whole suite + print(f" Go SAST scan failed: {e}") + return 1 + r = gs.score(findings) + gs.print_report(r) + return 0 if gs.passed(r) else 1 + + def main() -> int: ap = argparse.ArgumentParser() ap.add_argument("targets", nargs="*", help="target names (default: vampi + sast-injection)") @@ -554,9 +589,12 @@ def main() -> int: REPORT_DIR = args.report_dir by_name = {t.name: t for t in TARGETS} - docker_names, want_sast, unknown = resolve_selection(args.targets, args.all) + docker_names, want_sast, want_go_sast, unknown = resolve_selection( + args.targets, args.all + ) if unknown: - print(f"Unknown targets: {unknown}. Available: {list(by_name) + [SAST_INJECTION]}") + print(f"Unknown targets: {unknown}. " + f"Available: {list(by_name) + [SAST_INJECTION, GO_SAST]}") return 2 selected = [by_name[n] for n in docker_names] @@ -569,6 +607,7 @@ def main() -> int: for r in results ) sast_rc = run_sast_injection() if want_sast else 0 + go_sast_rc = run_go_sast() if want_go_sast else 0 if regressions: print(f"\n✗ {regressions} regression failure(s) — a finding the scanner " f"reliably catches was dropped by the full scan. See ⚠ REGRESSION above.") @@ -576,6 +615,9 @@ def main() -> int: if sast_rc: print("\n✗ SAST injection benchmark failed (recall < 100% or FP > 0).") return 1 + if go_sast_rc: + print("\n✗ Go SAST benchmark failed (class recall < 100%).") + return 1 return 0 diff --git a/benchmarks/sast_injection.py b/benchmarks/sast_injection.py index 273205a..8a79cbd 100644 --- a/benchmarks/sast_injection.py +++ b/benchmarks/sast_injection.py @@ -42,7 +42,8 @@ # they don't collide (e.g. "dom" would match "domain"/"random"). First hit wins. _TITLE_CUES = [ ("dom-xss", ("into the dom", "innerhtml", "document.write", "dom xss")), - ("reflected-xss", ("reflected", "html response")), + ("reflected-xss", ("reflected", "html response", "cross-site scripting", + "auto-escaping")), ("ssti", ("template injection", "ssti", "server-side template")), ("ssrf", ("outbound request", "ssrf")), ("path-traversal", ("filesystem write", "path traversal", "request-derived")), diff --git a/isitsecure/engine/code_analysis/go_route_mapper.py b/isitsecure/engine/code_analysis/go_route_mapper.py index fde8eaa..611c6c0 100644 --- a/isitsecure/engine/code_analysis/go_route_mapper.py +++ b/isitsecure/engine/code_analysis/go_route_mapper.py @@ -65,12 +65,21 @@ class GoRouteMapper: # `recv.Use(Middleware())` — router/group-wide middleware application. USE_PATTERN = re.compile(r"""(\w+)\.Use\s*\(([^)]*)\)""", re.MULTILINE) - # A middleware/handler name that names authentication. Deliberately - # auth-specific — a bare `.Use(` is also logging/CORS/recovery, which must - # not credit a route with auth. + # A middleware name that names authentication. Deliberately auth-specific — + # a bare `.Use(` is also logging/CORS/recovery, which must not credit auth. _AUTH_NAME = re.compile( r"(?i)(auth|login|jwt|token|session|protected|require[-_]?(?:auth|login))" ) + # An auth-naming middleware *applied as a call* — the name is immediately + # followed by `(`, so it matches a wrapper like `RequireAuth(h)` but NOT a + # handler merely *named* with an auth-ish word (`loginViewHandler`, + # `getSessionData`). Marking a route authed off a handler name would be the + # dangerous direction: it suppresses a genuine missing-auth finding. + _AUTH_MIDDLEWARE_CALL = re.compile( + r"(?i)\b\w*" + r"(?:auth|login|jwt|token|session|protected|require[-_]?(?:auth|login))" + r"\w*\s*\(" + ) # A handler is credited with auth only when its body both looks up an # identity AND refuses — checked by the route analyzer via handler_source; # the mapper only decides the definite router/group-middleware case itself. @@ -109,19 +118,20 @@ def _extract_routes(self, file_path: str, content: str) -> list[RouteEntry]: groups = self._group_prefixes(content) auth_receivers = self._auth_receivers(content) - def _entry(recv: str, methods: list[str], path: str, body: str): + def _entry(recv, methods, path, body, wrapper_auth): full = normalize_route_pattern(self._prefix(groups, recv) + path) - # A router/group with auth middleware guards every route on it — - # that is definite. Otherwise leave the per-route verdict to the - # route analyzer, which re-examines handler_source for an - # identity-check-AND-refusal (and the LSP tracer follows helpers - # across files). None of `content`'s other handlers can vouch here. + # A route is authed when a router/group applies auth middleware, OR + # an auth-naming middleware wraps the handler on the mount line + # (idiomatic Go: `r.GET(p, RequireAuth(h))`). Both are definite. + # Otherwise leave the per-route verdict to the route analyzer, which + # re-examines handler_source for an identity-check-AND-refusal (and + # the LSP tracer follows helpers across files). group_auth = recv in auth_receivers return RouteEntry( file_path=file_path, http_methods=methods, route_pattern=full, - has_auth_check=True if group_auth else False, + has_auth_check=True if (group_auth or wrapper_auth) else False, content=content, handler_source=body, ) @@ -131,20 +141,20 @@ def _entry(recv: str, methods: list[str], path: str, body: str): path = match.group(3) or match.group(4) # A real route registration passes a handler after the path — this # separates `r.GET("/p", h)` from `r.Header.Get("X")` / `c.Get("u")`. - is_route, body = self._resolve_handler(content, match.end()) + is_route, body, wrapper_auth = self._resolve_handler(content, match.end()) if not is_route: continue - routes.append(_entry(recv, [verb.upper()], path, body)) + routes.append(_entry(recv, [verb.upper()], path, body, wrapper_auth)) for match in self.HANDLE_PATTERN.finditer(content): recv = match.group(1) path = match.group(2) or match.group(3) - is_route, body = self._resolve_handler(content, match.end()) + is_route, body, wrapper_auth = self._resolve_handler(content, match.end()) if not is_route: continue # net/http muxes accept any method — wildcard so every verb reads # as reachable. - routes.append(_entry(recv, ["ANY"], path, body)) + routes.append(_entry(recv, ["ANY"], path, body, wrapper_auth)) return routes @@ -158,25 +168,68 @@ def _auth_receivers(self, content: str) -> set[str]: receivers.add(recv) return receivers - def _resolve_handler(self, content: str, after: int) -> tuple[bool, str]: + def _resolve_handler(self, content: str, after: int) -> tuple[bool, str, bool]: """Resolve the handler following a route registration's path arg. - Returns ``(is_route, handler_source)``: - - named handler ``, getUser)`` → (True, body of ``func getUser``) — or - (True, "") when it is package-qualified/defined elsewhere (the LSP - tracer resolves those cross-file); - - inline ``, func(...) {...}`` → (True, the literal's body); - - no handler (``r.Header.Get("X")``, ``c.Get("user")``) → (False, ""). + Returns ``(is_route, handler_source, wrapper_auth)``: + - named handler ``, getUser)`` → body of ``func getUser`` (or "" when it + is package-qualified/defined elsewhere; the LSP tracer resolves those); + - inline ``, func(...) {...}`` → the literal's body; + - wrapped ``, Logging(RequireAuth(getUser))`` → the innermost resolvable + handler's body, and ``wrapper_auth`` True when an auth-naming + middleware appears anywhere in the wrapper chain (idiomatic Go); + - no handler (``r.Header.Get("X")``, ``c.Get("user")``) → (False, "", False). + + The registration arguments are captured by balancing parentheses from + the path to the enclosing call's close, so nested wrappers are handled + without hard-coding any framework's middleware names. """ - tail = content[after:after + 160] - m = re.match(r"\s*,\s*([A-Za-z_]\w*(?:\.\w+)?)\s*[),]", tail) - if m: - return True, self._func_body(content, m.group(1)) - inline = re.match(r"\s*,\s*func\s*\(", tail) + args = self._call_args_tail(content, after) + arg = args.lstrip().lstrip(",").strip() + if not arg: + return False, "", False + + # Auth only from a middleware *call* wrapping the handler, never from a + # handler merely named with an auth-ish word. + wrapper_auth = bool(self._AUTH_MIDDLEWARE_CALL.search(args)) + + inline = re.match(r"\s*,\s*func\s*\(", args) if inline: brace = content.find("{", after + inline.end()) - return True, self._brace_block(content, brace) if brace != -1 else "" - return False, "" + body = self._brace_block(content, brace) if brace != -1 else "" + return True, body, wrapper_auth + + # Named or wrapped: the innermost identifier that resolves to a + # `func ` in this file is the real handler; wrappers resolve to "" + # and are skipped. Package-qualified names (pkg.H) resolve to "" too. + body = "" + for name in reversed(re.findall(r"[A-Za-z_]\w*", arg)): + resolved = self._func_body(content, name) + if resolved: + body = resolved + break + return True, body, wrapper_auth + + @staticmethod + def _call_args_tail(content: str, after: int) -> str: + """Text of the registration's remaining args, from just after the path + to the enclosing call's matching ``)`` (exclusive). + + ``after`` sits inside the verb/HandleFunc call (one paren already open), + so balancing from depth 1 yields ``, handlerExpr`` however deeply the + handler is wrapped. Bounded so a malformed file can't run away. + """ + depth = 1 + end = min(len(content), after + 2000) + for i in range(after, end): + ch = content[i] + if ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + if depth == 0: + return content[after:i] + return content[after:end] @classmethod def _func_body(cls, content: str, handler: str) -> str: diff --git a/isitsecure/engine/code_analysis/semgrep_rules/injection-go.yaml b/isitsecure/engine/code_analysis/semgrep_rules/injection-go.yaml index dc41609..e28a69d 100644 --- a/isitsecure/engine/code_analysis/semgrep_rules/injection-go.yaml +++ b/isitsecure/engine/code_analysis/semgrep_rules/injection-go.yaml @@ -69,6 +69,25 @@ rules: - pattern: $DB.QueryRowContext($CTX, $Q) - pattern: $DB.ExecContext($CTX, $Q) + # ---- XSS: unescaped data cast to a template safe-string type ---- + # html/template auto-escapes by default; casting a value to template.HTML/JS/ + # HTMLAttr/CSS/URL explicitly BYPASSES that escaping, so a non-literal cast is + # the canonical Go XSS sink (gosec G203). A string-literal argument is the + # intentional, safe case and is excluded, so this does not false-positive on + # static markup. + # Regex, not an AST pattern, on purpose: `template.HTML(x)` is a Go type + # CONVERSION whose AST match needs type resolution, which fails on a cloned + # repo whose modules aren't downloaded. The regex requires a non-quote, + # non-close-paren first argument char, so a string-literal cast + # (`template.HTML("...")`) — the intentional, safe case — is excluded and + # this does not false-positive on static markup. + - id: isitsecure-go-xss-template-safestring + languages: [go] + severity: ERROR + metadata: {category: injection_risk, isitsecure-severity: high} + message: "Unescaped data cast to a template safe-string type (template.HTML/JS/HTMLAttr/CSS/URL) bypasses Go's auto-escaping — cross-site scripting (XSS)." + pattern-regex: 'template\.(?:HTML|JS|HTMLAttr|CSS|URL)\s*\(\s*[^")\s]' + # ---- Command injection: user input → shell (intra-file taint) ---- # os/exec runs the program directly (no shell) UNLESS invoked as sh -c, so a # tainted arg is only injection when the command is a shell. Sink focuses on diff --git a/tests/benchmarks/test_sast_injection_scorer.py b/tests/benchmarks/test_sast_injection_scorer.py index d147e03..ea74050 100644 --- a/tests/benchmarks/test_sast_injection_scorer.py +++ b/tests/benchmarks/test_sast_injection_scorer.py @@ -207,28 +207,80 @@ def _import(self): self.rb = rb def test_default_runs_vampi_plus_sast(self): - docker, want_sast, unknown = self.rb.resolve_selection([], all_flag=False) + docker, want_sast, want_go, unknown = self.rb.resolve_selection([], all_flag=False) assert want_sast is True + assert want_go is False # go-sast is opt-in, not in the default set assert set(docker) == {"vampi-vulnerable", "vampi-secure"} assert unknown == [] def test_sast_only(self): - docker, want_sast, unknown = self.rb.resolve_selection(["sast-injection"], all_flag=False) + docker, want_sast, want_go, unknown = self.rb.resolve_selection(["sast-injection"], all_flag=False) assert docker == [] and want_sast is True and unknown == [] def test_docker_target_only_skips_sast(self): - docker, want_sast, unknown = self.rb.resolve_selection(["juiceshop"], all_flag=False) + docker, want_sast, want_go, unknown = self.rb.resolve_selection(["juiceshop"], all_flag=False) assert docker == ["juiceshop"] and want_sast is False def test_mixed_docker_and_sast(self): - docker, want_sast, unknown = self.rb.resolve_selection( + docker, want_sast, want_go, unknown = self.rb.resolve_selection( ["juiceshop", "sast-injection"], all_flag=False) assert docker == ["juiceshop"] and want_sast is True def test_all_flag_includes_sast(self): - docker, want_sast, unknown = self.rb.resolve_selection([], all_flag=True) + docker, want_sast, want_go, unknown = self.rb.resolve_selection([], all_flag=True) assert want_sast is True and len(docker) == len(self.rb.TARGETS) + assert want_go is True def test_unknown_target_reported(self): - docker, want_sast, unknown = self.rb.resolve_selection(["nope"], all_flag=False) + docker, want_sast, want_go, unknown = self.rb.resolve_selection(["nope"], all_flag=False) assert unknown == ["nope"] and docker == [] + + +class TestGoSastWiring: + """go-sast pseudo-target dispatch + scorer (benchmarks/go_sast.py).""" + + @pytest.fixture(autouse=True) + def _import(self): + import run_benchmarks as rb # noqa: E402 + import go_sast as gs # noqa: E402 + self.rb, self.gs = rb, gs + + def test_go_sast_named_runs_only_it(self): + docker, want_sast, want_go, unknown = self.rb.resolve_selection( + ["go-sast"], all_flag=False) + assert want_go is True and docker == [] and unknown == [] + + def test_go_sast_not_in_default_set(self): + _, _, want_go, _ = self.rb.resolve_selection([], all_flag=False) + assert want_go is False + + def test_all_flag_includes_go_sast(self): + _, _, want_go, _ = self.rb.resolve_selection([], all_flag=True) + assert want_go is True + + def test_score_full_recall(self): + findings = [ + {"category": "injection_risk", "title": "User input flows into a raw SQL query"}, + {"category": "injection_risk", "title": "Unescaped data cast to template.HTML — XSS"}, + {"category": "dependency_vuln", "title": "Vulnerable dependency: gin@1.6.3"}, + {"category": "auth_weakness", "title": "API route missing authentication check"}, + ] + r = self.gs.score(findings) + assert r["recall"] == {"found": 4, "total": 4} + assert self.gs.passed(r) + + def test_score_missing_xss_is_a_gap(self): + findings = [ + {"category": "injection_risk", "title": "raw SQL query"}, + {"category": "dependency_vuln", "title": "Vulnerable dependency"}, + {"category": "auth_weakness", "title": "missing authentication"}, + ] + r = self.gs.score(findings) + assert r["by_class"]["xss"]["detected"] is False + assert not self.gs.passed(r) + + def test_category_dict_shape_is_handled(self): + """Findings may carry category as an enum-dict {'value': ...}.""" + findings = [{"category": {"value": "injection_risk"}, "title": "sql injection"}] + r = self.gs.score(findings) + assert r["by_class"]["sqli"]["detected"] is True diff --git a/tests/engine/test_code_analysis/test_go_route_mapper.py b/tests/engine/test_code_analysis/test_go_route_mapper.py index 0342623..56e3a6a 100644 --- a/tests/engine/test_code_analysis/test_go_route_mapper.py +++ b/tests/engine/test_code_analysis/test_go_route_mapper.py @@ -143,6 +143,54 @@ def test_non_route_method_calls_are_not_routes(tmp_path): assert set(routes) == {"/real"} +def test_wrapped_middleware_handler_is_mapped(tmp_path): + """`router.GET(p, Log(AuthCheck(h)))` — a wrapped handler — is a route, and + the auth-naming wrapper marks it authed (idiomatic Go middleware chaining).""" + _write(tmp_path, "app.go", """ +package main +func setup(router *httprouter.Router) { + router.GET("/dash", mw.LoggingMiddleware(mw.AuthCheck(dashHandler))) + router.GET("/setup", mw.LoggingMiddleware(setupHandler)) +} +""") + routes = _by_pattern(GoRouteMapper().map_routes(str(tmp_path))) + assert "/dash" in routes and "/setup" in routes + assert routes["/dash"].has_auth_check is True # AuthCheck wrapper + assert routes["/setup"].has_auth_check is False # only logging + + +def test_handler_named_with_auth_word_is_not_auto_authed(tmp_path): + """A handler merely NAMED with an auth-ish word (loginViewHandler, + getSessionData) must NOT be treated as guarded — that would suppress a real + missing-auth. Auth is credited only from a wrapping middleware CALL.""" + _write(tmp_path, "app.go", """ +package main +func setup(router *httprouter.Router) { + router.GET("/login", mw.LoggingMiddleware(loginViewHandler)) + router.GET("/session", mw.LoggingMiddleware(getSessionData)) +} +""") + routes = _by_pattern(GoRouteMapper().map_routes(str(tmp_path))) + assert routes["/login"].has_auth_check is False + assert routes["/session"].has_auth_check is False + + +def test_wrapped_handler_body_resolves_to_innermost(tmp_path): + """handler_source is the innermost real handler's body, not a wrapper.""" + _write(tmp_path, "app.go", """ +package main +import "net/http" +func dashHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + w.Write([]byte("dash")) +} +func setup(router *httprouter.Router) { + router.GET("/dash", mw.Logging(mw.AuthCheck(dashHandler))) +} +""") + routes = _by_pattern(GoRouteMapper().map_routes(str(tmp_path))) + assert "func dashHandler" in routes["/dash"].handler_source + + def test_inline_handler_is_a_route(tmp_path): """An inline func literal handler still counts as a route.""" _write(tmp_path, "inline.go", """