Merge pull request #2362 from erik-krogh/promiseAll

Approved by max-schaefer

javascript/ql/src/Statements/UseOfReturnlessFunction.ql

@@ -68,7 +68,7 @@ predicate benignContext(Expr e) {
   any(InvokeExpr invoke).getCallee() = e
   or
   // arguments to Promise.resolve (and promise library variants) are benign. 
-  e = any(ResolvedPromiseDefinition promise).getValue().asExpr()
+  e = any(PromiseCreationCall promise).getValue().asExpr()
 }
 
 predicate oneshotClosure(DataFlow::CallNode call) {
@@ -198,7 +198,7 @@ module Deferred {
   /**
    * A resolved promise created by a `new Deferred().resolve()` call.
    */
-  class ResolvedDeferredPromiseDefinition extends ResolvedPromiseDefinition {
+  class ResolvedDeferredPromiseDefinition extends PromiseCreationCall {
     ResolvedDeferredPromiseDefinition() {
       this = any(DeferredPromiseDefinition def).ref().getAMethodCall("resolve")
     }

javascript/ql/src/semmle/javascript/Promises.qll

@@ -30,6 +30,22 @@ module Bluebird {
 
     override DataFlow::Node getValue() { result = getArgument(0) }
   }
+  
+  /**
+   * An aggregated promise produced either by `Promise.all`, `Promise.race` or `Promise.map`. 
+   */
+  class AggregateBluebirdPromiseDefinition extends PromiseCreationCall {
+    AggregateBluebirdPromiseDefinition() {
+      exists(string m | m = "all" or m = "race" or m = "map" | 
+        this = bluebird().getAMemberCall(m)
+      )
+    }
+
+    override DataFlow::Node getValue() {
+      result = getArgument(0).getALocalSource().(DataFlow::ArrayCreationNode).getAnElement()
+    }
+  }
+  
 }
 
 /**

javascript/ql/src/semmle/javascript/StandardLibrary.qll

@@ -152,15 +152,20 @@ private class ES2015PromiseDefinition extends PromiseDefinition, DataFlow::NewNo
 }
 
 /**
- * A promise that is resolved with the given value.
+ * A promise that is created and resolved with one or more value.
  */
-abstract class ResolvedPromiseDefinition extends DataFlow::CallNode {
+abstract class PromiseCreationCall extends DataFlow::CallNode {
   /**
    * Gets the value this promise is resolved with.
    */
   abstract DataFlow::Node getValue();
 }
 
+/**
+ * A promise that is created using a `.resolve()` call. 
+ */
+abstract class ResolvedPromiseDefinition extends PromiseCreationCall {}
+
 /**
  * A resolved promise created by the standard ECMAScript 2015 `Promise.resolve` function.
  */
@@ -172,6 +177,21 @@ class ResolvedES2015PromiseDefinition extends ResolvedPromiseDefinition {
   override DataFlow::Node getValue() { result = getArgument(0) }
 }
 
+/**
+ * An aggregated promise produced either by `Promise.all` or `Promise.race`. 
+ */
+class AggregateES2015PromiseDefinition extends PromiseCreationCall {
+  AggregateES2015PromiseDefinition() {
+    exists(string m | m = "all" or m = "race" | 
+      this = DataFlow::globalVarRef("Promise").getAMemberCall(m)
+    )
+  }
+
+  override DataFlow::Node getValue() {
+    result = getArgument(0).getALocalSource().(DataFlow::ArrayCreationNode).getAnElement()
+  }
+}
+
 /**
  * A data flow edge from a promise reaction to the corresponding handler.
  */
@@ -197,7 +217,7 @@ predicate promiseTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
   pred = succ.(PromiseDefinition).getResolveParameter().getACall().getArgument(0)
   or
   // from `x` to `Promise.resolve(x)`
-  pred = succ.(ResolvedPromiseDefinition).getValue()
+  pred = succ.(PromiseCreationCall).getValue()
   or
   exists(DataFlow::MethodCallNode thn, DataFlow::FunctionNode cb |
     thn.getMethodName() = "then" and cb = thn.getCallback(0)

javascript/ql/test/library-tests/Promises/ResolvedPromiseDefinition.qll

@@ -1,7 +1,7 @@
 import javascript
 
 query predicate test_ResolvedPromiseDefinition(
-  ResolvedPromiseDefinition resolved, DataFlow::Node res
+  PromiseCreationCall resolved, DataFlow::Node res
 ) {
   res = resolved.getValue()
 }

javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.js

@@ -88,6 +88,8 @@
 	}
 	
 	new Deferred().resolve(onlySideEffects()); // OK
+	
+	Promise.all([onlySideEffects(), onlySideEffects()])
 })();
 
 +function() {
@@ -104,4 +106,4 @@ class Bar extends Foo {
 	constructor() {
 		console.log(super()); // OK.
 	}
-}
+}