Merge pull request #4134 from erik-krogh/genCalls

Approved by asgerf

Author: codeql-ci

javascript/ql/src/javascript.qll

@@ -27,6 +27,7 @@ import semmle.javascript.Extend
 import semmle.javascript.Externs
 import semmle.javascript.Files
 import semmle.javascript.Functions
+import semmle.javascript.Generators
 import semmle.javascript.GlobalAccessPaths
 import semmle.javascript.HTML
 import semmle.javascript.HtmlSanitizers

javascript/ql/src/semmle/javascript/Functions.qll

@@ -204,6 +204,9 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
   /** Holds if this function is an asynchronous function. */
   predicate isAsync() { isAsync(this) }
 
+  /** Holds if this function is asynchronous or a generator. */
+  predicate isAsyncOrGenerator() { isAsync() or isGenerator() }
+
   /** Gets the enclosing function or toplevel of this function. */
   override StmtContainer getEnclosingContainer() { result = getEnclosingStmt().getContainer() }
 

javascript/ql/src/semmle/javascript/Generators.qll

@@ -0,0 +1,37 @@
+/**
+ * Provides classes and predicates for working with generator functions.
+ */
+
+import javascript
+private import semmle.javascript.dataflow.internal.PreCallGraphStep
+
+/**
+ * Classes and predicates for modelling data-flow for generator functions.
+ */
+private module GeneratorDataFlow {
+  private import DataFlow::PseudoProperties
+
+  private class ArrayIteration extends PreCallGraphStep {
+    override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
+      exists(DataFlow::FunctionNode f | f.getFunction().isGenerator() |
+        prop = iteratorElement() and
+        exists(YieldExpr yield |
+          yield.getContainer() = f.getFunction() and not yield.isDelegating()
+        |
+          pred.asExpr() = yield.getOperand()
+        ) and
+        succ = f.getReturnNode()
+      )
+    }
+
+    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
+      exists(DataFlow::FunctionNode f | f.getFunction().isGenerator() |
+        prop = iteratorElement() and
+        exists(YieldExpr yield | yield.getContainer() = f.getFunction() and yield.isDelegating() |
+          pred.asExpr() = yield.getOperand()
+        ) and
+        succ = f.getReturnNode()
+      )
+    }
+  }
+}

javascript/ql/src/semmle/javascript/dataflow/Configuration.qll

@@ -1031,7 +1031,7 @@ private predicate storeStep(
   summary = PathSummary::level()
   or
   exists(Function f, DataFlow::Node mid, DataFlow::Node invk |
-    not f.isAsync() and invk = succ
+    not f.isAsyncOrGenerator() and invk = succ
     or
     // store in an immediately awaited function call
     f.isAsync() and
@@ -1156,7 +1156,7 @@ private predicate loadStep(
   summary = PathSummary::level()
   or
   exists(Function f, DataFlow::Node read, DataFlow::Node invk |
-    not f.isAsync() and invk = succ
+    not f.isAsyncOrGenerator() and invk = succ
     or
     // load from an immediately awaited function call
     f.isAsync() and

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

@@ -1495,7 +1495,7 @@ module DataFlow {
     )
     or
     // from returned expr to the FunctionReturnNode.
-    exists(Function f | not f.isAsync() |
+    exists(Function f | not f.isAsyncOrGenerator() |
       DataFlow::functionReturnNode(succ, f) and pred = valueNode(f.getAReturnedExpr())
     )
   }

javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll

@@ -243,7 +243,7 @@ private module NodeTracking {
     basicStoreStep(pred, succ, prop) and
     summary = PathSummary::level()
     or
-    exists(Function f, DataFlow::Node mid | not f.isAsync() |
+    exists(Function f, DataFlow::Node mid | not f.isAsyncOrGenerator() |
       // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
       // and `succ` is an invocation of `f`
       reachableFromInput(f, succ, pred, mid, summary) and
@@ -266,7 +266,7 @@ private module NodeTracking {
     basicLoadStep(pred, succ, prop) and
     summary = PathSummary::level()
     or
-    exists(Function f, DataFlow::SourceNode parm | not f.isAsync() |
+    exists(Function f, DataFlow::SourceNode parm | not f.isAsyncOrGenerator() |
       argumentPassing(succ, pred, f, parm) and
       reachesReturn(f, parm.getAPropertyRead(prop), summary)
     )

javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll

@@ -71,6 +71,8 @@ predicate localExceptionStep(DataFlow::Node pred, DataFlow::Node succ) {
  */
 predicate localExceptionStepWithAsyncFlag(DataFlow::Node pred, DataFlow::Node succ, boolean async) {
   exists(DataFlow::Node target | target = getThrowTarget(pred) |
+    // this also covers generators - as the behavior of exceptions is close enough to the behavior of ordinary
+    // functions when it comes to exceptions (assuming that the iterator does not cross function boundaries).
     async = false and
     succ = target and
     not succ = any(DataFlow::FunctionNode f | f.getFunction().isAsync()).getExceptionalReturn()

javascript/ql/test/library-tests/Generators/DataFlow.expected

@@ -0,0 +1,12 @@
+import javascript
+import testUtilities.ConsistencyChecking
+
+class GeneratorFlowConfig extends DataFlow::Configuration {
+  GeneratorFlowConfig() { this = "GeneratorFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source.asExpr().getStringValue() = "source" }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument()
+  }
+}

javascript/ql/test/library-tests/Generators/DataFlow.ql

@@ -0,0 +1,53 @@
+(function () {
+  var source = "source";
+  sink(source); // NOT OK
+
+  function *gen1() {
+    yield source;
+  }
+  for (const x of gen1()) {
+    sink(x); // NOT OK
+  }
+
+  function *gen2() {
+    yield "safe";
+    return source;
+  }
+  sink(gen2()); // OK
+
+  Array.from(gen1()).forEach(x => sink(x)); // NOT OK
+
+  function gen3() {
+    yield source;
+  }
+  Array.from(gen3()).forEach(x => sink(x)); // NOT OK
+
+  function *gen4() {
+    throw source;
+  }
+  try {
+    Array.from(gen4());
+  } catch (e) {
+    sink(e); // NOT OK
+  }
+
+  function *delegating() {
+    yield* delegate();
+  }
+
+  function *delegate() {
+    yield source;
+  }
+
+  Array.from(delegating()).forEach(x => sink(x)); // NOT OK
+
+  function *delegating2() {
+    yield* returnsTaint();
+  }
+
+  function returnsTaint() {
+    return source;
+  }
+
+  Array.from(delegating2()).forEach(x => sink(x)); // OK
+});