mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 11:49:28 +00:00
fix(rules): Node vm is an eval sink; a package name is not a sink token
mongo-express CVE-2019-10758 passes the request body to vm.runInNewContext. No
injection sink named that call, so the flow was answered and missed. runInNewContext,
runInContext, runInThisContext and compileFunction are now CWE-95 sinks.
Once the file's questions completed, the fixed pin showed a second defect. Every call
on `require('mongodb-query-parser')` has that package name as its full name. The
bounded-token match treats `-` as a boundary, so the name matched the SQL sink
`query`. In a full name, `-` belongs to a package name, so it no longer separates
tokens.
Exploratory (retune after protocol-v1): mongo-express vulnerable pin now detected at
lib/bson.js:60.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
dbcb230990
commit
ec3d1457fb
@@ -121,6 +121,12 @@
|
||||
def mentionsToken(code: String, tokens: List[String]): Boolean =
|
||||
tokens.exists(t => boundedPattern(t).matcher(code).find())
|
||||
|
||||
// A full name is not source text: a `-` in it is part of an npm package's name, never an operator.
|
||||
// `require('mongodb-query-parser')` gives every call on the parser the full name `mongodb-query-parser...`,
|
||||
// and read as code that mentions the SQL sink `query` -- mongo-express's fixed pin was reported twice for
|
||||
// parsing a string with the very library its fix switched to.
|
||||
def fullNameMentions(fullName: String, token: String): Boolean = mentionsToken(fullName.replace('-', '_'), List(token))
|
||||
|
||||
val sourcePatterns = split(sourcesS)
|
||||
val sinkNames = split(sinksS)
|
||||
val sanitizerNames = split(sanitizersS)
|
||||
@@ -314,7 +320,7 @@
|
||||
def familySinkCall(node: io.shiftleft.codepropertygraph.generated.nodes.AstNode): Boolean = node match {
|
||||
case c: io.shiftleft.codepropertygraph.generated.nodes.Call if !c.name.startsWith("<operator") =>
|
||||
val callee = c.code.takeWhile(_ != '(')
|
||||
sinkNames.exists(n => c.name == n || mentionsToken(callee, List(n)) || mentionsToken(c.methodFullName, List(n)))
|
||||
sinkNames.exists(n => c.name == n || mentionsToken(callee, List(n)) || fullNameMentions(c.methodFullName, n))
|
||||
case _ => false
|
||||
}
|
||||
|
||||
@@ -701,7 +707,7 @@
|
||||
|
||||
def sinkMatches(c: io.shiftleft.codepropertygraph.generated.nodes.Call, n: String): Boolean = {
|
||||
val operator = c.name.startsWith("<operator")
|
||||
c.name == n || (!operator && (mentionsToken(calleeText(c.code), List(n)) || mentionsToken(c.methodFullName, List(n))))
|
||||
c.name == n || (!operator && (mentionsToken(calleeText(c.code), List(n)) || fullNameMentions(c.methodFullName, n)))
|
||||
}
|
||||
|
||||
// Does THIS path element cleanse the value flowing through it?
|
||||
|
||||
@@ -11,6 +11,15 @@ cwe = "CWE-95"
|
||||
calls = ["eval"]
|
||||
rule_ids = ["js-eval"]
|
||||
|
||||
[[sink]]
|
||||
# CWE-95. Node's `vm` module compiles and runs a string exactly as `eval` does -- its own documentation says
|
||||
# it is not a security mechanism. mongo-express CVE-2019-10758 is `vm.runInNewContext('doc = eval((' +
|
||||
# string + '));')` over a request body, and with only `eval` listed the flow ended at a sink nobody named.
|
||||
id = "vm.eval"
|
||||
cwe = "CWE-95"
|
||||
calls = ["runInNewContext", "runInContext", "runInThisContext", "compileFunction"]
|
||||
rule_ids = []
|
||||
|
||||
[[sink]]
|
||||
id = "exec"
|
||||
cwe = "CWE-78"
|
||||
|
||||
@@ -963,3 +963,58 @@ def test_a_field_the_method_just_overwrote_is_not_object_state(tmp_path: Path) -
|
||||
assert flows["find"], "the method whose own write carries request data must be found"
|
||||
assert not flows["deleteMe"], f"a field the method had just overwritten was read as object state: {flows['deleteMe']}"
|
||||
assert flows["rerun"], "a method reading the state another method left must still be found"
|
||||
|
||||
|
||||
@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine")
|
||||
def test_node_vm_is_an_eval_and_a_package_name_is_not_a_sink(tmp_path: Path) -> None:
|
||||
"""mongo-express CVE-2019-10758 ran the request body through `vm.runInNewContext`, which no sink named. Its fix
|
||||
parses with `mongodb-query-parser`, and every call on that parser carries the package name in its full name --
|
||||
which read as the SQL sink `query`. The shipped injection sinks are used, not a hand-picked list."""
|
||||
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")["injection"].sinks
|
||||
src = tmp_path / "src"
|
||||
src.mkdir()
|
||||
(src / "bson.js").write_text(
|
||||
"const vm = require('vm');\n"
|
||||
"const parser = require('mongodb-query-parser');\n"
|
||||
"const db = require('./db');\n"
|
||||
"function vulnerable(req, res) { const doc = req.body.document;\n"
|
||||
" return vm.runInNewContext('doc = eval((' + doc + '));', {}); }\n"
|
||||
"function parsed(req, res) { return parser(req.body.document) + parser.toJSString(req.query.x, ' '); }\n"
|
||||
"function raw(req, res) { return db.query(req.query.q); }\n"
|
||||
)
|
||||
cpg = tmp_path / "cpg.bin"
|
||||
built = subprocess.run([frontend, str(src), "-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:]}")
|
||||
common = {"sources": "req.body,req.query", "sinks": ",".join(sinks), "file": "bson.js"}
|
||||
requests = tmp_path / "requests.json"
|
||||
requests.write_text(json.dumps({name: {**common, "function": name} for name in ("vulnerable", "parsed", "raw")}))
|
||||
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,
|
||||
cwd=str(tmp_path),
|
||||
)
|
||||
assert done.returncode == 0, (done.stderr or done.stdout or "")[-500:]
|
||||
payload = extract_payload(done.stdout or "")
|
||||
assert isinstance(payload, dict)
|
||||
flows = {rid: [r for r in payload[rid] if isinstance(r, dict) and r.get("sink")] for rid in ("vulnerable", "parsed", "raw")}
|
||||
assert any("runInNewContext" in str(r["sink"]) for r in flows["vulnerable"]), flows["vulnerable"]
|
||||
assert not flows["parsed"], f"a package name was read as a sink: {flows['parsed']}"
|
||||
assert any("query" in str(r["sink"]) for r in flows["raw"]), f"a real query call was lost: {flows['raw']}"
|
||||
|
||||
Reference in New Issue
Block a user