mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(semantic-facts): give untrusted destination the clients a Node server calls
The family census recorded this family completing 44 questions on NodeGoat, finding the open redirect, and having no way to ask about the repository's documented SSRF: needle.get on a query-string URL at app/routes/research.js:16. A thin vocabulary is a silent false negative. 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 covers its verb methods too, since a token matches word-bounded against the callee and axios is bounded inside axios.get; axios.get and axios.post are dropped as subsumed rather than unwanted. http and https stay dotted because their namespace also holds createServer, and a bare token would have claimed every server a repository starts. The 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, every other family is unchanged, and 299 of 299 questions still complete. 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. A handler calling axios.get with a literal and needle.get with req.query.url reports the tainted client alone, and it 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. 1,411 tests pass, eleven skipped; seven Joern-gated query tests pass in the image. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
This commit is contained in:
co-authored by
Claude Opus 5
parent
4e2619e248
commit
57f5a3f0dc
@@ -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
|
assignment node. Nothing about this changes the release gates. No capability is qualified by
|
||||||
these subjects.
|
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).
|
||||||
|
|||||||
@@ -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 `<operator>.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.
|
- _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 `<operator>.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_
|
- _Boundary: contributor-scan detection, Evaluation harness_
|
||||||
- _Requirements: 6.2, 6.3, 8.1_
|
- _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.
|
- **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.
|
- 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.
|
- 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_
|
- _Boundary: shipped semantic facts_
|
||||||
- _Depends: 14.1_
|
- _Depends: 14.1_
|
||||||
- _Requirements: 6.2, 8.1_
|
- _Requirements: 6.2, 8.1_
|
||||||
|
|||||||
@@ -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:<lambda>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:<lambda>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:<lambda>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:<lambda>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:<lambda>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:<lambda>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"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -66,9 +66,37 @@ rule_ids = []
|
|||||||
|
|
||||||
[[sink]]
|
[[sink]]
|
||||||
# CWE-918 / CWE-601: outbound requests and redirects to a caller-supplied destination.
|
# 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"
|
id = "outbound.request"
|
||||||
cwe = "CWE-918"
|
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 = []
|
rule_ids = []
|
||||||
|
|
||||||
[[sink]]
|
[[sink]]
|
||||||
|
|||||||
@@ -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"]
|
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 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}"
|
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}"
|
||||||
|
|||||||
Reference in New Issue
Block a user