diff --git a/.kiro/specs/pre-push-safety-net/status.md b/.kiro/specs/pre-push-safety-net/status.md index 6c33b92..52ed5d9 100644 --- a/.kiro/specs/pre-push-safety-net/status.md +++ b/.kiro/specs/pre-push-safety-net/status.md @@ -152,4 +152,11 @@ the word `set` in a code comment, matched because the sink text clause searched assignment node. Nothing about this changes the release gates. No capability is qualified by these subjects. -Evidence: [family detection census](../../../benchmarks/measurements/2026-09-19-family-detection-census.json). +The first of the three gaps is closed. The untrusted-destination vocabulary now holds the +client surface a Node server actually calls, and NodeGoat's SSRF is reported at +`app/routes/research.js:16` with a witness naming the call. One data change, no query change, +every other family unchanged. Output encoding and access control remain open as tasks 14.3 +and 14.4. + +Evidence: [family detection census](../../../benchmarks/measurements/2026-09-19-family-detection-census.json), +[untrusted destination vocabulary](../../../benchmarks/measurements/2026-09-19-untrusted-destination-vocabulary.json). diff --git a/.kiro/specs/pre-push-safety-net/tasks.md b/.kiro/specs/pre-push-safety-net/tasks.md index 736358d..c3bc2f9 100644 --- a/.kiro/specs/pre-push-safety-net/tasks.md +++ b/.kiro/specs/pre-push-safety-net/tasks.md @@ -471,10 +471,13 @@ Preserve all completed task evidence below. - _The census found one false positive and it is fixed._ NodeGoat's only prototype report was the word `set` inside the comment `// set these up in case we have an error case`: the matched node is the `.assignment` holding the whole 1,000-character signup handler, and its text was searched entire. `sinkMatches` now confines its text clause to the callee and excludes operator nodes, which is the fourth instance of this bug class after `fgets`/`gets`, `resolve`/`resolveUrl` and `user`/`users`. Measured on NodeGoat's non-vendor calls: injection sinks 28 to 3, prototype 13 to 2, untrusted_destination 26 to 7, with no true positive lost. `req.query` had been an injection sink, because `query` is word-bounded inside it. - _Boundary: contributor-scan detection, Evaluation harness_ - _Requirements: 6.2, 6.3, 8.1_ -- [ ] 14.2 Give untrusted destination the sinks a Node server actually calls +- [x] 14.2 Give untrusted destination the sinks a Node server actually calls - **Named by the 14.1 census.** NodeGoat's documented SSRF is `needle.get(req.query.url + req.query.symbol)` at `app/routes/research.js:16`, and `needle.get` is absent from a vocabulary holding axios, fetch, got, `http.get` and request. The family works and found the open redirect in the same repository; it could not ask about the request. - This is a vocabulary gap, so fix it as data rather than as a query change, and state which client libraries the declared set now covers. - Done when the research handler's request is asked about, with a witness naming the call, and no benign client call is reported. + - _Verified 2026-09-19:_ 1,411 tests passed, eleven skipped; seven Joern-gated query tests pass in the image; Ruff/format/mypy passed. One data change, no query change. The declared set now covers axios, got, needle, request, superagent, unirest, undici and fetch as bare module tokens, plus `http.get`, `http.request`, `https.get` and `https.request` dotted. Bare is the documented call form of each module and also covers its verb methods, because a token matches word-bounded against the callee and `axios` is bounded inside `axios.get`; `axios.get` and `axios.post` were therefore dropped as subsumed. `http` and `https` stay dotted because their namespace also holds `createServer`, and a bare token would have claimed every server a repository starts. + - The research handler's request is now reported at `app/routes/research.js:16`, model_entailed, witness `req.query.url + req.query.symbol -> needle.get(url, ...)`. This family goes from one finding to two on NodeGoat and every other family is unchanged, with 299 of 299 questions still completing. Measured on that graph, `needle` matches exactly one call and the eleven other clients match none, in first-party or vendored code, so NodeGoat shows the gap closing and cannot show those clients' noise. + - A control asserts both halves rather than only the recall half: a handler calling `axios.get` with a literal and `needle.get` with `req.query.url` reports the tainted client alone, and the control reads the shipped vocabulary rather than a copy so removing the client fails it. The CWE-601 redirect entry is untouched; a Location-header redirect is value-shaped and stays outside what this family asks. Evidence: `benchmarks/measurements/2026-09-19-untrusted-destination-vocabulary.json`. - _Boundary: shipped semantic facts_ - _Depends: 14.1_ - _Requirements: 6.2, 8.1_ diff --git a/benchmarks/measurements/2026-09-19-untrusted-destination-vocabulary.json b/benchmarks/measurements/2026-09-19-untrusted-destination-vocabulary.json new file mode 100644 index 0000000..0335390 --- /dev/null +++ b/benchmarks/measurements/2026-09-19-untrusted-destination-vocabulary.json @@ -0,0 +1,186 @@ +{ + "schema_version": 1, + "experiment": "untrusted-destination-vocabulary-v1", + "date": "2026-09-19", + "task": "pre-push-safety-net 14.2", + "claim_scope": "One data change to the shipped JavaScript sink vocabulary, measured on NodeGoat. Detection only. No capability qualified: NodeGoat developed this analyser.", + "question": "Does the untrusted-destination family ask about the outbound request a Node server actually makes?", + "before": [ + "axios.get", + "axios.post", + "fetch", + "request", + "got", + "http.get", + "https.get" + ], + "after": [ + "axios", + "got", + "needle", + "request", + "superagent", + "unirest", + "undici", + "fetch", + "http.get", + "http.request", + "https.get", + "https.request" + ], + "design": { + "bare_module_names": "axios, got, needle, request, superagent, unirest, undici and fetch are the documented call form of their own module, and a bare token also covers every verb method through it: tokens match word-bounded against the callee, and axios is bounded inside axios.get. So axios.get and axios.post were dropped as subsumed, not as unwanted.", + "dotted_node_core": "http and https stay dotted. Their namespace also holds createServer, and a bare token would have claimed every server a repository starts as an outbound request.", + "redirect_unchanged": "The CWE-601 entry is untouched. res.writeHead with a Location header is a value-shaped redirect this arbiter cannot decide, and widening to writeHead would claim every response." + }, + "measured_on_nodegoat": { + "token_matches_first_party": { + "needle": 1, + "redirect": 7, + "axios": 0, + "got": 0, + "request": 0, + "superagent": 0, + "unirest": 0, + "undici": 0, + "fetch": 0, + "http.get": 0, + "http.request": 0, + "https.get": 0, + "https.request": 0, + "location.assign": 0, + "location.replace": 0 + }, + "token_matches_vendored": "zero for every candidate, including the 13 vendored bundles under app/assets", + "note": "needle matches exactly one call, the research handler's request. redirect matches seven, of which six take a literal destination and are correctly not reported. The other clients do not appear in this repository at all, so NodeGoat shows the gap closing and cannot show these clients' noise." + }, + "census_effect": { + "family": "untrusted_destination", + "questions_asked": 44, + "findings_before": 1, + "findings_after": 2, + "new_finding": { + "site": "app/routes/research.js:16:0", + "rung": "model_entailed", + "witness": "req.query.url + req.query.symbol -> needle.get(url, (error, newResponse, body) => {..." + }, + "other_families_unchanged": true, + "all_questions_completed": "299 of 299" + }, + "control": "tests/test_cpg_queries_compile.py::test_a_literal_destination_is_not_an_untrusted_destination builds a handler that calls axios.get with a literal and needle.get with req.query.url, and asserts the flow is reported for the tainted client only. It reads the shipped vocabulary rather than a copy, so removing the client fails the control. Seven Joern-gated query tests pass in the image.", + "limits": [ + "NodeGoat contains only one of the twelve clients, so eleven are declared on documentation, not measured.", + "A bare module token matches any call whose callee spells that module, including a same-named local variable.", + "Server-side redirects written as a Location header are still outside what this family asks." + ], + "full_census_after": { + "access_control": { + "regions_offering": 35, + "questions_asked": 35, + "questions_completed": 35, + "findings": 0, + "rungs": {}, + "examples": [], + "verdict": "answered, nothing found" + }, + "config_secrets": { + "regions_offering": 44, + "questions_asked": 44, + "questions_completed": 44, + "findings": 1, + "rungs": { + "model_entailed": 1 + }, + "examples": [ + { + "site": "server.js:78:?", + "rung": "model_entailed", + "witness": "session({\n // genid: (req) => {\n // return genuuid() // use UUIDs for session IDs\n //},\n secret: cookieSecret,\n // Both mandatory in Express v4\n saveUninit", + "reached_from": 1 + } + ], + "verdict": "establishes findings" + }, + "injection": { + "regions_offering": 44, + "questions_asked": 44, + "questions_completed": 44, + "findings": 3, + "rungs": { + "model_entailed": 3 + }, + "examples": [ + { + "site": "app/routes/contributions.js:32:2", + "rung": "model_entailed", + "witness": "req.body.preTax -> eval(req.body.preTax) in app/routes/contributions.js (line 32, 1 steps)", + "reached_from": 1 + }, + { + "site": "app/routes/contributions.js:33:2", + "rung": "model_entailed", + "witness": "req.body.afterTax -> eval(req.body.afterTax) in app/routes/contributions.js (line 33, 1 steps)", + "reached_from": 1 + }, + { + "site": "app/routes/contributions.js:34:2", + "rung": "model_entailed", + "witness": "req.body.roth -> eval(req.body.roth) in app/routes/contributions.js (line 34, 1 steps)", + "reached_from": 1 + } + ], + "verdict": "establishes findings" + }, + "output_encoding": { + "regions_offering": 44, + "questions_asked": 44, + "questions_completed": 44, + "findings": 0, + "rungs": {}, + "examples": [], + "verdict": "answered, nothing found" + }, + "path": { + "regions_offering": 44, + "questions_asked": 44, + "questions_completed": 44, + "findings": 0, + "rungs": {}, + "examples": [], + "verdict": "answered, nothing found" + }, + "prototype": { + "regions_offering": 44, + "questions_asked": 44, + "questions_completed": 44, + "findings": 0, + "rungs": {}, + "examples": [], + "verdict": "answered, nothing found" + }, + "untrusted_destination": { + "regions_offering": 44, + "questions_asked": 44, + "questions_completed": 44, + "findings": 2, + "rungs": { + "model_entailed": 2 + }, + "examples": [ + { + "site": "app/routes/index.js:72:1", + "rung": "model_entailed", + "witness": "req.query.url -> res.redirect(req.query.url) in app/routes/index.js (line 72, 1 steps)", + "reached_from": 1 + }, + { + "site": "app/routes/research.js:16:0", + "rung": "model_entailed", + "witness": "req.query.url + req.query.symbol -> needle.get(url, (error, newResponse, body) => {\n if (!error && newResponse.statusCode === 200) {\n res.writeHead(200, {\n ", + "reached_from": 1 + } + ], + "verdict": "establishes findings" + } + } +} diff --git a/src/openultrasast/ruleset/semantic/javascript.toml b/src/openultrasast/ruleset/semantic/javascript.toml index cf571ce..fb7fe3c 100644 --- a/src/openultrasast/ruleset/semantic/javascript.toml +++ b/src/openultrasast/ruleset/semantic/javascript.toml @@ -66,9 +66,37 @@ rule_ids = [] [[sink]] # CWE-918 / CWE-601: outbound requests and redirects to a caller-supplied destination. +# +# The set shipped with five clients and missed the one the measured repository calls. NodeGoat's documented +# SSRF is `needle.get(req.query.url + req.query.symbol)` at `app/routes/research.js:16`, and the 2026-09-19 +# family census recorded this family completing 44 questions there, finding the open redirect two files away, +# and having no way to ask about the request. A thin vocabulary is a silent false negative, so what follows is +# the client surface a Node server actually calls. +# +# Bare where the module ITSELF is the request function, which is the documented call form for all of these +# (`axios(config)`, `got(url)`, `needle('get', url)`, `request(opts)`), and which also covers every verb +# method through it: a token is matched word-bounded against the callee, and `axios` is bounded inside +# `axios.get`. Dotted for `http` and `https`, whose namespace also holds `createServer` -- a server is not an +# outbound request, and the bare form would have claimed every one of them. +# +# Measured on NodeGoat: `needle` matches exactly the research handler's request and nothing else; none of the +# other clients appears in that repository, in first-party code or vendored. id = "outbound.request" cwe = "CWE-918" -calls = ["axios.get", "axios.post", "fetch", "request", "got", "http.get", "https.get"] +calls = [ + "axios", # axios, and axios.get/.post/.put/.patch/.delete/.head/.request through it + "got", # got + "needle", # needle + "request", # request / request-promise + "superagent", # superagent + "unirest", # unirest + "undici", # undici, and undici.fetch/.request through it + "fetch", # global fetch, node-fetch, cross-fetch + "http.get", + "http.request", + "https.get", + "https.request", +] rule_ids = [] [[sink]] diff --git a/tests/test_cpg_queries_compile.py b/tests/test_cpg_queries_compile.py index 9f75aee..99e069f 100644 --- a/tests/test_cpg_queries_compile.py +++ b/tests/test_cpg_queries_compile.py @@ -133,3 +133,61 @@ def test_a_sink_word_in_a_comment_is_not_a_sink(tmp_path: Path) -> None: inventory = [row for row in rows if row.get("kind") == "operation_inventory_complete"] assert inventory, f"taint.sc reported no inventory row: {rows[:3]}" assert all(int(row.get("operations", 0)) == 0 for row in inventory), f"a commented sink word was counted as an operation: {inventory}" + + +@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine") +def test_a_literal_destination_is_not_an_untrusted_destination(tmp_path: Path) -> None: + """The client the request flows into is reported; the client called with a literal is not. + + The family census found this family answering 44 questions on NodeGoat while unable to ask about its + documented SSRF, because `needle` was absent from a vocabulary holding axios, fetch, got and request. + Widening a vocabulary is how a recall gap is closed and also how a false positive is created, so both + halves are asserted here against the shipped token set rather than a copy of it. + """ + from openultrasast.cpg.backend import extract_payload + from openultrasast.model.specs import taint_specs + + frontend = __import__("shutil").which("jssrc2cpg") + if frontend is None: + pytest.skip("jssrc2cpg is not installed") + sinks = taint_specs(language="javascript")["untrusted_destination"].sinks + assert "needle" in sinks, "the shipped vocabulary no longer holds the client this control is about" + + source = tmp_path / "research.js" + source.write_text( + 'const axios = require("axios");\n' + 'const needle = require("needle");\n' + "\n" + "function handler(req, res) {\n" + ' axios.get("https://api.example.com/health");\n' + " return needle.get(req.query.url, (error, response, body) => res.end(body));\n" + "}\n" + ) + cpg = tmp_path / "cpg.bin" + built = subprocess.run([frontend, str(tmp_path), "-o", str(cpg)], capture_output=True, text=True, timeout=600, check=False) + if not cpg.is_file(): + pytest.skip(f"could not build a sample cpg: {(built.stderr or '')[-200:]}") + + requests = tmp_path / "requests.json" + requests.write_text(json.dumps({"0": {"sources": "req.query", "sinks": ",".join(sinks), "function": "handler"}})) + done = subprocess.run( + [ + _joern() or "joern", + "--script", + str((QUERIES / "taint.sc").resolve()), + "--param", + f"cpgFile={cpg}", + "--param", + f"requestsFile={requests}", + ], + capture_output=True, + text=True, + timeout=900, + check=False, + ) + assert done.returncode == 0, f"taint.sc did not run: {(done.stderr or done.stdout or '')[-500:]}" + payload = extract_payload(done.stdout or "") + assert payload is not None, "taint.sc produced no parseable payload" + flows = [row for row in (payload.get("0") or []) if row.get("sink")] + assert flows, "the request flowing into the client was not reported at all" + assert all("needle" in str(row["sink"]) for row in flows), f"a client called with a literal was reported: {flows}"