From 18a1946be8d6ce5e09f33c26098bc76e80e13225 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 14:37:10 +0200 Subject: [PATCH 01/10] JS: Factor out some big-steps --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 1a96e25b3b9f..af3ead09ded8 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1277,18 +1277,13 @@ module API { boundArgs = 0 and prop = "" or - exists(Promisify::PromisifyCall promisify | - trackUseNode(nd, false, boundArgs, prop, t.continue()).flowsTo(promisify.getArgument(0)) and - promisified = true and - prop = "" and - result = promisify - ) + promisificationBigStep(trackUseNode(nd, false, boundArgs, prop, t.continue()), result) and + promisified = true and + prop = "" or - exists(DataFlow::PartialInvokeNode pin, DataFlow::Node pred, int predBoundArgs | - trackUseNode(nd, promisified, predBoundArgs, prop, t.continue()).flowsTo(pred) and - prop = "" and - result = pin.getBoundFunction(pred, boundArgs - predBoundArgs) and - boundArgs in [0 .. 10] + exists(DataFlow::SourceNode pred, int predBoundArgs | + pred = trackUseNode(nd, promisified, predBoundArgs, prop, t.continue()) and + partialInvocationBigStep(pred, result, boundArgs - predBoundArgs) ) or exists(DataFlow::SourceNode mid | @@ -1296,11 +1291,11 @@ module API { AdditionalUseStep::step(pragma[only_bind_out](mid), result) ) or - exists(DataFlow::Node pred, string preprop | - trackUseNode(nd, promisified, boundArgs, preprop, t.continue()).flowsTo(pred) and + exists(DataFlow::SourceNode pred, string preprop | + pred = trackUseNode(nd, promisified, boundArgs, preprop, t.continue()) and + loadStoreBigStep(pred, result, prop) and promisified = false and - boundArgs = 0 and - SharedTypeTrackingStep::loadStoreStep(pred, result, prop) + boundArgs = 0 | prop = preprop or @@ -1310,6 +1305,28 @@ module API { t = useStep(nd, promisified, boundArgs, prop, result) } + pragma[nomagic] + private predicate promisificationBigStep(DataFlow::SourceNode node1, DataFlow::SourceNode node2) { + exists(Promisify::PromisifyCall promisify | + node1 = promisify.getArgument(0).getALocalSource() and + node2 = promisify + ) + } + + pragma[nomagic] + private predicate partialInvocationBigStep( + DataFlow::SourceNode node1, DataFlow::SourceNode node2, int boundArgs + ) { + node2 = any(DataFlow::PartialInvokeNode pin).getBoundFunction(node1.getALocalUse(), boundArgs) + } + + pragma[nomagic] + private predicate loadStoreBigStep( + DataFlow::SourceNode node1, DataFlow::SourceNode node2, string prop + ) { + SharedTypeTrackingStep::loadStoreStep(node1.getALocalUse(), node2, prop) + } + /** * Holds if `nd`, which is a use of an API-graph node, flows in zero or more potentially * inter-procedural steps to some intermediate node, and then from that intermediate node to From f805e265a82fc8de0c05e1f3b2af5a3151cf14ed Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 14:47:40 +0200 Subject: [PATCH 02/10] JS: Eliminate some redundant type checks --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index af3ead09ded8..54f7e58ebcb3 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1253,6 +1253,25 @@ module API { private import semmle.javascript.dataflow.TypeTracking + /** + * A version of `DataFlow::SourceNode` without a charpred, to avoid redundant type checks that the + * optimizer is not yet able to remove. Should only be used in cases where we know only `SourceNode` values + * will appear. + */ + private class RawSourceNode extends DataFlow::Node { + predicate flowsTo(DataFlow::Node node) { this.(DataFlow::SourceNode).flowsTo(node) } + + predicate flowsToExpr(Expr expr) { this.(DataFlow::SourceNode).flowsToExpr(expr) } + + DataFlow::PropRead getAPropertyRead(string prop) { + result = this.(DataFlow::SourceNode).getAPropertyRead(prop) + } + + DataFlow::PropWrite getAPropertyWrite() { + result = this.(DataFlow::SourceNode).getAPropertyWrite() + } + } + /** * Gets a data-flow node to which `nd`, which is a use of an API-graph node, flows. * @@ -1266,9 +1285,8 @@ module API { * - `prop`: if non-empty, the flow is only guaranteed to preserve the value of this property, * and not necessarily the entire object. */ - private DataFlow::SourceNode trackUseNode( - DataFlow::SourceNode nd, boolean promisified, int boundArgs, string prop, - DataFlow::TypeTracker t + private RawSourceNode trackUseNode( + RawSourceNode nd, boolean promisified, int boundArgs, string prop, DataFlow::TypeTracker t ) { t.start() and use(_, nd) and @@ -1347,8 +1365,8 @@ module API { ) } - private DataFlow::SourceNode trackUseNode( - DataFlow::SourceNode nd, boolean promisified, int boundArgs, string prop + private RawSourceNode trackUseNode( + RawSourceNode nd, boolean promisified, int boundArgs, string prop ) { result = trackUseNode(nd, promisified, boundArgs, prop, DataFlow::TypeTracker::end()) } @@ -1357,9 +1375,7 @@ module API { * Gets a node that is inter-procedurally reachable from `nd`, which is a use of some node. */ cached - DataFlow::SourceNode trackUseNode(DataFlow::SourceNode nd) { - result = trackUseNode(nd, false, 0, "") - } + RawSourceNode trackUseNode(RawSourceNode nd) { result = trackUseNode(nd, false, 0, "") } private DataFlow::SourceNode trackDefNode(DataFlow::Node nd, DataFlow::TypeBackTracker t) { t.start() and From 7db4c4285d88076b6dc0043aa615c0286cbb15b7 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 14:51:39 +0200 Subject: [PATCH 03/10] JS: Prepare promisification and partial invocation steps for 'noopt' --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 54f7e58ebcb3..ed3a0f6d1699 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1295,13 +1295,27 @@ module API { boundArgs = 0 and prop = "" or - promisificationBigStep(trackUseNode(nd, false, boundArgs, prop, t.continue()), result) and - promisified = true and - prop = "" - or - exists(DataFlow::SourceNode pred, int predBoundArgs | - pred = trackUseNode(nd, promisified, predBoundArgs, prop, t.continue()) and - partialInvocationBigStep(pred, result, boundArgs - predBoundArgs) + exists( + RawSourceNode prev, boolean prevPromisified, int prevBoundArgs, string prevProp, + DataFlow::TypeTracker prevT + | + prev = trackUseNode(nd, prevPromisified, prevBoundArgs, prevProp, prevT) + | + promisificationBigStep(prev, result) and + prevPromisified = false and + prevProp = "" and + promisified = prevPromisified and + boundArgs = prevBoundArgs and + prop = prevProp and + t = prevT.continue() + or + exists(int b | + partialInvocationBigStep(prev, result, b) and + boundArgs = prevBoundArgs + b and + promisified = prevPromisified and + prop = prevProp and + t = prevT.continue() + ) ) or exists(DataFlow::SourceNode mid | From 9f001f8892160c5ad40a3da185104c20e2866de2 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 15:01:57 +0200 Subject: [PATCH 04/10] JS:More 'noopt' preparations --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index ed3a0f6d1699..0ad518919786 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1295,45 +1295,37 @@ module API { boundArgs = 0 and prop = "" or - exists( - RawSourceNode prev, boolean prevPromisified, int prevBoundArgs, string prevProp, - DataFlow::TypeTracker prevT - | - prev = trackUseNode(nd, prevPromisified, prevBoundArgs, prevProp, prevT) + exists(RawSourceNode prev, boolean prevPromisified, int prevBoundArgs, string prevProp | + prev = trackUseNode(nd, prevPromisified, prevBoundArgs, prevProp, t) | promisificationBigStep(prev, result) and prevPromisified = false and prevProp = "" and promisified = prevPromisified and boundArgs = prevBoundArgs and - prop = prevProp and - t = prevT.continue() + prop = prevProp or exists(int b | partialInvocationBigStep(prev, result, b) and boundArgs = prevBoundArgs + b and promisified = prevPromisified and - prop = prevProp and - t = prevT.continue() + prop = prevProp ) - ) + or + loadStoreBigStep(prev, result, prop) and + prevPromisified = false and + prevBoundArgs = 0 and + promisified = prevPromisified and + boundArgs = prevBoundArgs and + prevProp = [prop, ""] + ) and + t.end() // 't' must be a valid ending point for the above cases (i.e. not inside a content) or exists(DataFlow::SourceNode mid | mid = trackUseNode(nd, promisified, boundArgs, prop, t) and AdditionalUseStep::step(pragma[only_bind_out](mid), result) ) or - exists(DataFlow::SourceNode pred, string preprop | - pred = trackUseNode(nd, promisified, boundArgs, preprop, t.continue()) and - loadStoreBigStep(pred, result, prop) and - promisified = false and - boundArgs = 0 - | - prop = preprop - or - preprop = "" - ) - or t = useStep(nd, promisified, boundArgs, prop, result) } From 938938c47032b7707b5c4a7c65c74e306159362c Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 15:10:19 +0200 Subject: [PATCH 05/10] JS: Use pragma[noopt] to force join order in trackUsenode --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 0ad518919786..988998056d78 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1285,11 +1285,12 @@ module API { * - `prop`: if non-empty, the flow is only guaranteed to preserve the value of this property, * and not necessarily the entire object. */ + pragma[noopt] private RawSourceNode trackUseNode( RawSourceNode nd, boolean promisified, int boundArgs, string prop, DataFlow::TypeTracker t ) { - t.start() and use(_, nd) and + t.start() and result = nd and promisified = false and boundArgs = 0 and @@ -1299,8 +1300,7 @@ module API { prev = trackUseNode(nd, prevPromisified, prevBoundArgs, prevProp, t) | promisificationBigStep(prev, result) and - prevPromisified = false and - prevProp = "" and + validPromisificationState(prevPromisified, prevProp) and promisified = prevPromisified and boundArgs = prevBoundArgs and prop = prevProp @@ -1313,8 +1313,7 @@ module API { ) or loadStoreBigStep(prev, result, prop) and - prevPromisified = false and - prevBoundArgs = 0 and + validLoadStoreBigState(prevPromisified, prevBoundArgs) and promisified = prevPromisified and boundArgs = prevBoundArgs and prevProp = [prop, ""] @@ -1323,12 +1322,23 @@ module API { or exists(DataFlow::SourceNode mid | mid = trackUseNode(nd, promisified, boundArgs, prop, t) and - AdditionalUseStep::step(pragma[only_bind_out](mid), result) + AdditionalUseStep::step(mid, result) ) or t = useStep(nd, promisified, boundArgs, prop, result) } + pragma[nomagic] + private predicate validPromisificationState(boolean promisified, string prop) { + promisified = false and + prop = "" + } + + pragma[nomagic] + private predicate validLoadStoreBigState(boolean promisified, int boundArgs) { + promisified = false and boundArgs = 0 + } + pragma[nomagic] private predicate promisificationBigStep(DataFlow::SourceNode node1, DataFlow::SourceNode node2) { exists(Promisify::PromisifyCall promisify | From f541c8b657659cecb82ff86ac6c74142f9d74f69 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 15:33:30 +0200 Subject: [PATCH 06/10] JS: Treat steps between SourceNodes as LevelStep() --- .../semmle/javascript/dataflow/internal/StepSummary.qll | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll index fed492074b6a..4885db626c0f 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll @@ -67,7 +67,13 @@ private module Cached { */ cached predicate step(DataFlow::SourceNode pred, DataFlow::SourceNode succ, StepSummary summary) { - exists(DataFlow::Node mid | pred.flowsTo(mid) | StepSummary::smallstep(mid, succ, summary)) + exists(DataFlow::Node mid | + pred.flowsTo(mid) and + StepSummary::smallstep(mid, succ, summary) and + (pred = mid or not mid instanceof DataFlow::SourceNode) + ) + or + pred.flowsTo(succ) and summary = LevelStep() and pred != succ } pragma[nomagic] From 796a2bc4e77be1ff5b82fd05d291e3f111b46e36 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 15:49:30 +0200 Subject: [PATCH 07/10] Also use RawNode in AdditionalUseStep --- javascript/ql/lib/semmle/javascript/ApiGraphs.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 988998056d78..7c7433fa9e81 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1320,7 +1320,7 @@ module API { ) and t.end() // 't' must be a valid ending point for the above cases (i.e. not inside a content) or - exists(DataFlow::SourceNode mid | + exists(RawSourceNode mid | mid = trackUseNode(nd, promisified, boundArgs, prop, t) and AdditionalUseStep::step(mid, result) ) From 945fa6871c6d4fe6fe9d9e4a15735ce1f2542aa9 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 20 Oct 2025 15:49:45 +0200 Subject: [PATCH 08/10] JS: Inline useStep and special-case return steps to avoid fan-out --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 7c7433fa9e81..699d2b443c92 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1325,7 +1325,18 @@ module API { AdditionalUseStep::step(mid, result) ) or - t = useStep(nd, promisified, boundArgs, prop, result) + exists(RawSourceNode prev, DataFlow::TypeTracker prevT | + prev = trackUseNode(nd, promisified, boundArgs, prop, prevT) + | + exists(StepSummary summary | + restrictedBigStep(prev, result, summary) and + t = prevT.append(summary) + ) + or + t.hasCall() = false and + returnBigStep(prev, result) and + t = prevT + ) } pragma[nomagic] @@ -1361,26 +1372,23 @@ module API { SharedTypeTrackingStep::loadStoreStep(node1.getALocalUse(), node2, prop) } - /** - * Holds if `nd`, which is a use of an API-graph node, flows in zero or more potentially - * inter-procedural steps to some intermediate node, and then from that intermediate node to - * `res` in one step. The entire flow is described by the resulting `TypeTracker`. - * - * This predicate exists solely to enforce a better join order in `trackUseNode` above. - */ - pragma[noopt] - private DataFlow::TypeTracker useStep( - DataFlow::Node nd, boolean promisified, int boundArgs, string prop, DataFlow::Node res - ) { - exists(DataFlow::TypeTracker t, StepSummary summary, DataFlow::SourceNode prev | - prev = trackUseNode(nd, promisified, boundArgs, prop, t) and - StepSummary::step(prev, res, summary) and - result = t.append(summary) and + pragma[nomagic] + private predicate restrictedBigStep(DataFlow::Node node1, DataFlow::Node node2, StepSummary step) { + StepSummary::step(node1, node2, step) and + not ( // Block argument-passing into 'this' when it determines the call target - not summary = CallReceiverStep() + step = CallReceiverStep() + or + // Special-case return to avoid large fan-out + step = ReturnStep() ) } + pragma[nomagic] + private predicate returnBigStep(DataFlow::SourceNode node1, DataFlow::Node node2) { + StepSummary::step(node1, node2, ReturnStep()) + } + private RawSourceNode trackUseNode( RawSourceNode nd, boolean promisified, int boundArgs, string prop ) { From 795466519a5310dc1be59d7dccf67f248b266e26 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 21 Oct 2025 10:03:49 +0200 Subject: [PATCH 09/10] JS: More API graph optimizations --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 699d2b443c92..1615c067129a 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1041,7 +1041,7 @@ module API { ) or // property reads - exists(DataFlow::SourceNode src, DataFlow::SourceNode pred, string propDesc | + exists(RawSourceNode src, RawSourceNode pred, string propDesc | use(base, src) and pred = trackUseNode(src, false, 0, propDesc) and propertyRead(pred, propDesc, lbl, ref) and @@ -1050,9 +1050,7 @@ module API { (base instanceof TNonModuleDef or base instanceof TUse) ) or - exists(DataFlow::SourceNode src, DataFlow::SourceNode pred | - use(base, src) and pred = trackUseNode(src) - | + exists(RawSourceNode src, RawSourceNode pred | use(base, src) and pred = trackUseNode(src) | lbl = Label::instance() and ref = pred.getAnInstantiation() or @@ -1263,6 +1261,8 @@ module API { predicate flowsToExpr(Expr expr) { this.(DataFlow::SourceNode).flowsToExpr(expr) } + DataFlow::Node getALocalUse() { result = this.(DataFlow::SourceNode).getALocalUse() } + DataFlow::PropRead getAPropertyRead(string prop) { result = this.(DataFlow::SourceNode).getAPropertyRead(prop) } @@ -1270,6 +1270,14 @@ module API { DataFlow::PropWrite getAPropertyWrite() { result = this.(DataFlow::SourceNode).getAPropertyWrite() } + + DataFlow::InvokeNode getAnInvocation() { + result = this.(DataFlow::SourceNode).getAnInvocation() + } + + DataFlow::NewNode getAnInstantiation() { + result = this.(DataFlow::SourceNode).getAnInstantiation() + } } /** From f8b819760fc43d60fecabcaeed8423e329b50262 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 21 Oct 2025 10:19:20 +0200 Subject: [PATCH 10/10] JS: Set promisified=true after promisification --- javascript/ql/lib/semmle/javascript/ApiGraphs.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 1615c067129a..a0f4eaeaf4b9 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1309,7 +1309,7 @@ module API { | promisificationBigStep(prev, result) and validPromisificationState(prevPromisified, prevProp) and - promisified = prevPromisified and + promisified = true and boundArgs = prevBoundArgs and prop = prevProp or