Merge pull request github#4712 from asgerf/js/api-graph-tweaks

Approved by max-schaefer

Author: codeql-ci

javascript/ql/src/semmle/javascript/ApiGraphs.qll

@@ -54,17 +54,17 @@ module API {
     /**
      * Gets a call to the function represented by this API component.
      */
-    DataFlow::CallNode getACall() { result = getReturn().getAnImmediateUse() }
+    CallNode getACall() { result = getReturn().getAnImmediateUse() }
 
     /**
      * Gets a `new` call to the function represented by this API component.
      */
-    DataFlow::NewNode getAnInstantiation() { result = getInstance().getAnImmediateUse() }
+    NewNode getAnInstantiation() { result = getInstance().getAnImmediateUse() }
 
     /**
      * Gets an invocation (with our without `new`) to the function represented by this API component.
      */
-    DataFlow::InvokeNode getAnInvocation() { result = getACall() or result = getAnInstantiation() }
+    InvokeNode getAnInvocation() { result = getACall() or result = getAnInstantiation() }
 
     /**
      * Gets a data-flow node corresponding to the right-hand side of a definition of the API
@@ -82,6 +82,12 @@ module API {
      */
     DataFlow::Node getARhs() { Impl::rhs(this, result) }
 
+    /**
+     * Gets a data-flow node that may interprocedurally flow to the right-hand side of a definition
+     * of the API component represented by this node.
+     */
+    DataFlow::Node getAValueReachingRhs() { result = Impl::trackDefNode(getARhs()) }
+
     /**
      * Gets a node representing member `m` of this API component.
      *
@@ -114,11 +120,17 @@ module API {
      * For example, if this node represents a use of some class `A`, then there might be a node
      * representing instances of `A`, typically corresponding to expressions `new A()` at the
      * source level.
+     *
+     * This predicate may have multiple results when there are multiple constructor calls invoking this API component.
+     * Consider using `getAnInstantiation()` if there is a need to distinguish between individual constructor calls.
      */
     Node getInstance() { result = getASuccessor(Label::instance()) }
 
     /**
      * Gets a node representing the `i`th parameter of the function represented by this node.
+     *
+     * This predicate may have multiple results when there are multiple invocations of this API component.
+     * Consider using `getAnInvocation()` if there is a need to distingiush between individual calls.
      */
     bindingset[i]
     Node getParameter(int i) { result = getASuccessor(Label::parameter(i)) }
@@ -133,6 +145,9 @@ module API {
 
     /**
      * Gets a node representing the last parameter of the function represented by this node.
+     *
+     * This predicate may have multiple results when there are multiple invocations of this API component.
+     * Consider using `getAnInvocation()` if there is a need to distingiush between individual calls.
      */
     Node getLastParameter() { result = getParameter(getNumParameter() - 1) }
 
@@ -144,6 +159,10 @@ module API {
     /**
      * Gets a node representing a parameter or the receiver of the function represented by this
      * node.
+     *
+     * This predicate may result in a mix of parameters from different call sites in cases where
+     * there are multiple invocations of this API component.
+     * Consider using `getAnInvocation()` if there is a need to distingiush between individual calls.
      */
     Node getAParameter() {
       result = getASuccessor(Label::parameterByStringIndex(_)) or
@@ -152,6 +171,9 @@ module API {
 
     /**
      * Gets a node representing the result of the function represented by this node.
+     *
+     * This predicate may have multiple results when there are multiple invocations of this API component.
+     * Consider using `getACall()` if there is a need to distingiush between individual calls.
      */
     Node getReturn() { result = getASuccessor(Label::return()) }
 
@@ -871,6 +893,54 @@ module API {
   }
 
   import Label as EdgeLabel
+
+  /**
+   * An `InvokeNode` that is connected to the API graph.
+   *
+   * Can be used to reason about calls to an external API in which the correlation between
+   * parameters and/or return values must be retained.
+   *
+   * The member predicates `getParameter`, `getReturn`, and `getInstance` mimic the corresponding
+   * predicates from `API::Node`. These are guaranteed to exist and be unique to this call.
+   */
+  class InvokeNode extends DataFlow::InvokeNode {
+    API::Node callee;
+
+    InvokeNode() {
+      this = callee.getReturn().getAnImmediateUse() or
+      this = callee.getInstance().getAnImmediateUse()
+    }
+
+    /** Gets the API node for the `i`th parameter of this invocation. */
+    Node getParameter(int i) {
+      result = callee.getParameter(i) and
+      result.getARhs() = getArgument(i)
+    }
+
+    /** Gets the API node for a parameter of this invocation. */
+    Node getAParameter() { result = getParameter(_) }
+
+    /** Gets the API node for the last parameter of this invocation. */
+    Node getLastParameter() { result = getParameter(getNumArgument() - 1) }
+
+    /** Gets the API node for the return value of this call. */
+    Node getReturn() {
+      result = callee.getReturn() and
+      result.getAnImmediateUse() = this
+    }
+
+    /** Gets the API node for the object constructed by this invocation. */
+    Node getInstance() {
+      result = callee.getInstance() and
+      result.getAnImmediateUse() = this
+    }
+  }
+
+  /** A call connected to the API graph. */
+  class CallNode extends InvokeNode, DataFlow::CallNode { }
+
+  /** A `new` call connected to the API graph. */
+  class NewNode extends InvokeNode, DataFlow::NewNode { }
 }
 
 private module Label {

javascript/ql/src/semmle/javascript/frameworks/NoSQL.qll

@@ -62,22 +62,20 @@ private module MongoDB {
   }
 
   /** A call to a MongoDB query method. */
-  private class QueryCall extends DatabaseAccess, DataFlow::CallNode {
+  private class QueryCall extends DatabaseAccess, API::CallNode {
     int queryArgIdx;
-    API::Node callee;
 
     QueryCall() {
       exists(string method |
         CollectionMethodSignatures::interpretsArgumentAsQuery(method, queryArgIdx) and
-        callee = getACollection().getMember(method)
-      ) and
-      this = callee.getACall()
+        this = getACollection().getMember(method).getACall()
+      )
     }
 
     override DataFlow::Node getAQueryArgument() { result = getArgument(queryArgIdx) }
 
     DataFlow::Node getACodeOperator() {
-      result = getADollarWhereProperty(callee.getParameter(queryArgIdx))
+      result = getADollarWhereProperty(getParameter(queryArgIdx))
     }
   }
 
@@ -679,22 +677,26 @@ private module Minimongo {
   }
 
   /** A call to a Minimongo query method. */
-  private class QueryCall extends DatabaseAccess, DataFlow::MethodCallNode {
+  private class QueryCall extends DatabaseAccess, API::CallNode {
     int queryArgIdx;
-    API::Node callee;
 
     QueryCall() {
       exists(string m |
-        callee = API::moduleImport("minimongo").getAMember().getReturn().getAMember().getMember(m) and
-        this = callee.getACall() and
+        this =
+          API::moduleImport("minimongo")
+              .getAMember()
+              .getReturn()
+              .getAMember()
+              .getMember(m)
+              .getACall() and
         CollectionMethodSignatures::interpretsArgumentAsQuery(m, queryArgIdx)
       )
     }
 
     override DataFlow::Node getAQueryArgument() { result = getArgument(queryArgIdx) }
 
     DataFlow::Node getACodeOperator() {
-      result = getADollarWhereProperty(callee.getParameter(queryArgIdx))
+      result = getADollarWhereProperty(getParameter(queryArgIdx))
     }
   }
 
@@ -715,14 +717,13 @@ private module Minimongo {
  */
 private module MarsDB {
   /** A call to a MarsDB query method. */
-  private class QueryCall extends DatabaseAccess, DataFlow::MethodCallNode {
+  private class QueryCall extends DatabaseAccess, API::CallNode {
     int queryArgIdx;
-    API::Node callee;
 
     QueryCall() {
       exists(string m |
-        callee = API::moduleImport("marsdb").getMember("Collection").getInstance().getMember(m) and
-        this = callee.getACall() and
+        this =
+          API::moduleImport("marsdb").getMember("Collection").getInstance().getMember(m).getACall() and
         // implements parts of the Minimongo interface
         Minimongo::CollectionMethodSignatures::interpretsArgumentAsQuery(m, queryArgIdx)
       )
@@ -731,7 +732,7 @@ private module MarsDB {
     override DataFlow::Node getAQueryArgument() { result = getArgument(queryArgIdx) }
 
     DataFlow::Node getACodeOperator() {
-      result = getADollarWhereProperty(callee.getParameter(queryArgIdx))
+      result = getADollarWhereProperty(getParameter(queryArgIdx))
     }
   }
 

javascript/ql/test/ApiGraphs/call-nodes/index.js

@@ -0,0 +1,4 @@
+import { foo } from 'mylibrary';
+
+foo({ value: 1 }, { value: 1 });
+foo({ value: 2 }, { value: 2 });

javascript/ql/test/ApiGraphs/call-nodes/test.expected

@@ -0,0 +1,4 @@
+values
+| index.js:3:1:3:31 | foo({ v ... e: 1 }) | 1 | 1 |
+| index.js:4:1:4:31 | foo({ v ... e: 2 }) | 2 | 2 |
+mismatch

javascript/ql/test/ApiGraphs/call-nodes/test.ql

@@ -0,0 +1,19 @@
+import javascript
+
+class FooCall extends API::CallNode {
+  FooCall() { this = API::moduleImport("mylibrary").getMember("foo").getACall() }
+
+  DataFlow::Node getFirst() { result = getParameter(0).getMember("value").getARhs() }
+
+  DataFlow::Node getSecond() { result = getParameter(1).getMember("value").getARhs() }
+}
+
+query predicate values(FooCall call, int first, int second) {
+  first = call.getFirst().getIntValue() and
+  second = call.getSecond().getIntValue()
+}
+
+query predicate mismatch(FooCall call, string msg) {
+  call.getFirst().getIntValue() != call.getSecond().getIntValue() and
+  msg = "mismatching parameter indices found for call"
+}