From e40b93b8a353b3fcdd87d4ee4e4e10d08ac866c6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 29 Apr 2025 14:50:57 +0200 Subject: [PATCH 1/3] JS: Add type-tracking step through simple Promise.all() calls --- .../internal/flow_summaries/Promises.qll | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll index 33299a3f5c09..1122c38320a5 100644 --- a/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll +++ b/javascript/ql/lib/semmle/javascript/internal/flow_summaries/Promises.qll @@ -4,6 +4,7 @@ private import javascript private import semmle.javascript.dataflow.FlowSummary +private import semmle.javascript.dataflow.TypeTracking private import FlowSummaryUtil DataFlow::SourceNode promiseConstructorRef() { @@ -211,12 +212,57 @@ private class PromiseReject extends SummarizedCallable { } } +/** + * A call to `Promise.all()`. + */ +class PromiseAllCall extends DataFlow::CallNode { + PromiseAllCall() { this = promiseConstructorRef().getAMemberCall("all") } + + /** Gets the source of the input array */ + DataFlow::ArrayCreationNode getInputArray() { result = this.getArgument(0).getALocalSource() } + + /** Gets the `n`th element of the input array */ + DataFlow::Node getNthInput(int n) { result = this.getInputArray().getElement(n) } + + /** Gets a reference to the output array. */ + DataFlow::SourceNode getOutputArray() { + exists(AwaitExpr await | + this.flowsToExpr(await.getOperand()) and + result = await.flow() + ) + or + result = this.getAMethodCall("then").getCallback(0).getParameter(0) + } + + /** Gets the `n`th output */ + DataFlow::SourceNode getNthOutput(int n) { + exists(string prop | + result = this.getOutputArray().getAPropertyRead(prop) and + n = prop.toInt() + ) + } +} + +/** + * Helps type-tracking simple uses of `Promise.all()` such as `const [a, b] = await Promise.all([x, y])`. + * + * Due to limited access path depth, type tracking can't track things that are in a promise and an array + * at once. This generates a step directly from the input array to the output array. + */ +private class PromiseAllStep extends SharedTypeTrackingStep { + override predicate loadStep(DataFlow::Node node1, DataFlow::Node node2, string prop) { + exists(PromiseAllCall call, int n | + node1 = call.getNthInput(n) and + node2 = call.getNthOutput(n) and + prop = Promises::valueProp() + ) + } +} + private class PromiseAll extends SummarizedCallable { PromiseAll() { this = "Promise.all()" } - override DataFlow::InvokeNode getACallSimple() { - result = promiseConstructorRef().getAMemberCall("all") - } + override DataFlow::InvokeNode getACallSimple() { result instanceof PromiseAllCall } override predicate propagatesFlow(string input, string output, boolean preservesValue) { preservesValue = true and From eae1e1cb0225bf8cfa93549d4df7eceea8f38129 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 29 Apr 2025 14:51:17 +0200 Subject: [PATCH 2/3] JS: Make API graphs rely on type-tracking steps in general --- javascript/ql/lib/semmle/javascript/ApiGraphs.qll | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 974fdd7c0cbf..276fe5a01690 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -850,10 +850,10 @@ module API { ) or lbl = Label::promised() and - PromiseFlow::storeStep(rhs, pred, Promises::valueProp()) + SharedTypeTrackingStep::storeStep(rhs, pred, Promises::valueProp()) or lbl = Label::promisedError() and - PromiseFlow::storeStep(rhs, pred, Promises::errorProp()) + SharedTypeTrackingStep::storeStep(rhs, pred, Promises::errorProp()) or // The return-value of a getter G counts as a definition of property G // (Ordinary methods and properties are handled as PropWrite nodes) @@ -1008,11 +1008,11 @@ module API { propDesc = "" ) or - PromiseFlow::loadStep(pred.getALocalUse(), ref, Promises::valueProp()) and + SharedTypeTrackingStep::loadStep(pred.getALocalUse(), ref, Promises::valueProp()) and lbl = Label::promised() and (propDesc = Promises::valueProp() or propDesc = "") or - PromiseFlow::loadStep(pred.getALocalUse(), ref, Promises::errorProp()) and + SharedTypeTrackingStep::loadStep(pred.getALocalUse(), ref, Promises::errorProp()) and lbl = Label::promisedError() and (propDesc = Promises::errorProp() or propDesc = "") } From da5d7991521a243ae8accedf99b7c60bba2093ee Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 30 Apr 2025 11:59:47 +0200 Subject: [PATCH 3/3] JS: Change note --- javascript/ql/src/change-notes/2025-04-30-promise-all.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-04-30-promise-all.md diff --git a/javascript/ql/src/change-notes/2025-04-30-promise-all.md b/javascript/ql/src/change-notes/2025-04-30-promise-all.md new file mode 100644 index 000000000000..a50e31ea01d0 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-04-30-promise-all.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- +* Type information is now propagated more precisely through `Promise.all()` calls, + leading to more resolved calls and more sources and sinks being detected.