Merge branch 'main' into main

Author: im-soohyun

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll

@@ -31,4 +31,6 @@ module CppDataFlow implements InputSig<Location> {
   predicate viableImplInCallContext = Private::viableImplInCallContext/2;
 
   predicate neverSkipInPathGraph = Private::neverSkipInPathGraph/1;
+
+  int defaultFieldFlowBranchLimit() { result = 3 }
 }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

@@ -1652,8 +1652,6 @@ predicate validParameterAliasStep(Node node1, Node node2) {
   )
 }
 
-private predicate isTopLevel(Cpp::Stmt s) { any(Function f).getBlock().getAStmt() = s }
-
 private Cpp::Stmt getAChainedBranch(Cpp::IfStmt s) {
   result = s.getThen()
   or
@@ -1684,11 +1682,9 @@ private Instruction getAnInstruction(Node n) {
 }
 
 private newtype TDataFlowSecondLevelScope =
-  TTopLevelIfBranch(Cpp::Stmt s) {
-    exists(Cpp::IfStmt ifstmt | s = getAChainedBranch(ifstmt) and isTopLevel(ifstmt))
-  } or
+  TTopLevelIfBranch(Cpp::Stmt s) { s = getAChainedBranch(_) } or
   TTopLevelSwitchCase(Cpp::SwitchCase s) {
-    exists(Cpp::SwitchStmt switchstmt | s = switchstmt.getASwitchCase() and isTopLevel(switchstmt))
+    exists(Cpp::SwitchStmt switchstmt | s = switchstmt.getASwitchCase())
   }
 
 /**

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 = "")
     }

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

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.

misc/codegen/generators/qlgen.py

@@ -24,6 +24,7 @@
 import logging
 import pathlib
 import re
+import shutil
 import subprocess
 import typing
 import itertools
@@ -257,6 +258,15 @@ def format(codeql, files):
     if not ql_files:
         return
     format_cmd = [codeql, "query", "format", "--in-place", "--"] + ql_files
+    if "/" in codeql or "\\" in codeql:
+        if not pathlib.Path(codeql).exists():
+            raise FormatError(f"Provided CodeQL binary `{codeql}` does not exist")
+    else:
+        codeql_path = shutil.which(codeql)
+        if not codeql_path:
+            raise FormatError(
+                f"`{codeql}` not found in PATH. Either install it, or pass `-- --codeql-binary` with a full path")
+        codeql = codeql_path
     res = subprocess.run(format_cmd, stderr=subprocess.PIPE, text=True)
     if res.returncode:
         for line in res.stderr.splitlines():

misc/codegen/test/test_qlgen.py

@@ -52,6 +52,8 @@ def qlgen_opts(opts):
     opts.ql_format = True
     opts.root_dir = paths.root_dir
     opts.force = False
+    opts.codeql_binary = "./my_fake_codeql"
+    pathlib.Path(opts.codeql_binary).touch()
     return opts
 
 
@@ -499,7 +501,6 @@ def test_class_dir_imports(generate_import_list):
 
 
 def test_format(opts, generate, render_manager, run_mock):
-    opts.codeql_binary = "my_fake_codeql"
     run_mock.return_value.stderr = "some\nlines\n"
     render_manager.written = [
         pathlib.Path("x", "foo.ql"),
@@ -508,13 +509,12 @@ def test_format(opts, generate, render_manager, run_mock):
     ]
     generate([schema.Class('A')])
     assert run_mock.mock_calls == [
-        mock.call(["my_fake_codeql", "query", "format", "--in-place", "--", "x/foo.ql", "bar.qll"],
+        mock.call([opts.codeql_binary, "query", "format", "--in-place", "--", "x/foo.ql", "bar.qll"],
                   stderr=subprocess.PIPE, text=True),
     ]
 
 
 def test_format_error(opts, generate, render_manager, run_mock):
-    opts.codeql_binary = "my_fake_codeql"
     run_mock.return_value.stderr = "some\nlines\n"
     run_mock.return_value.returncode = 1
     render_manager.written = [
@@ -526,6 +526,24 @@ def test_format_error(opts, generate, render_manager, run_mock):
         generate([schema.Class('A')])
 
 
+def test_format_no_codeql(opts, generate, render_manager, run_mock):
+    pathlib.Path(opts.codeql_binary).unlink()
+    render_manager.written = [
+        pathlib.Path("bar.qll"),
+    ]
+    with pytest.raises(qlgen.FormatError):
+        generate([schema.Class('A')])
+
+
+def test_format_no_codeql_in_path(opts, generate, render_manager, run_mock):
+    opts.codeql_binary = "my_fake_codeql"
+    render_manager.written = [
+        pathlib.Path("bar.qll"),
+    ]
+    with pytest.raises(qlgen.FormatError):
+        generate([schema.Class('A')])
+
+
 @pytest.mark.parametrize("force", [False, True])
 def test_manage_parameters(opts, generate, renderer, force):
     opts.force = force

python/ql/src/Resources/FileNotAlwaysClosedQuery.qll

@@ -52,14 +52,29 @@ class FileWrapperCall extends DataFlow::CallCfgNode {
 abstract class FileClose extends DataFlow::CfgNode {
   /** Holds if this file close will occur if an exception is thrown at `raises`. */
   predicate guardsExceptions(DataFlow::CfgNode raises) {
-    this.asCfgNode() = raises.asCfgNode().getAnExceptionalSuccessor().getASuccessor*()
+    cfgGetASuccessorStar(raises.asCfgNode().getAnExceptionalSuccessor(), this.asCfgNode())
     or
     // The expression is after the close call.
     // This also covers the body of a `with` statement.
-    raises.asCfgNode() = this.asCfgNode().getASuccessor*()
+    cfgGetASuccessorStar(this.asCfgNode(), raises.asCfgNode())
   }
 }
 
+private predicate cfgGetASuccessor(ControlFlowNode src, ControlFlowNode sink) {
+  sink = src.getASuccessor()
+}
+
+pragma[inline]
+private predicate cfgGetASuccessorPlus(ControlFlowNode src, ControlFlowNode sink) =
+  fastTC(cfgGetASuccessor/2)(src, sink)
+
+pragma[inline]
+private predicate cfgGetASuccessorStar(ControlFlowNode src, ControlFlowNode sink) {
+  src = sink
+  or
+  cfgGetASuccessorPlus(src, sink)
+}
+
 /** A call to the `.close()` method of a file object. */
 class FileCloseCall extends FileClose {
   FileCloseCall() { exists(DataFlow::MethodCallNode mc | mc.calls(this, "close")) }