Merge pull request #4449 from max-schaefer/js/api-graphs-type-handling-improvements

Approved by erik-krogh

Author: codeql-ci

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

@@ -286,6 +286,17 @@ module API {
   /** Gets a node corresponding to an export of module `m`. */
   Node moduleExport(string m) { result = Impl::MkModuleDef(m).(Node).getMember("exports") }
 
+  /** Provides helper predicates for accessing API-graph nodes. */
+  module Node {
+    /** Gets a node whose type has the given qualified name. */
+    Node ofType(string moduleName, string exportedName) {
+      exists(TypeName tn |
+        tn.hasQualifiedName(moduleName, exportedName) and
+        result = Impl::MkCanonicalNameUse(tn).(Node).getInstance()
+      )
+    }
+  }
+
   /**
    * An API entry point.
    *
@@ -301,9 +312,6 @@ module API {
 
     /** Gets a data-flow node that defines this entry point. */
     abstract DataFlow::Node getARhs();
-
-    /** Gets an API-graph node for this entry point. */
-    API::Node getNode() { result = root().getASuccessor(this) }
   }
 
   /**
@@ -346,16 +354,42 @@ module API {
             exists(SSA::implicitInit([nm.getModuleVariable(), nm.getExportsVariable()]))
           )
         )
+        or
+        m = any(CanonicalName n | isDefined(n)).getExternalModuleName()
+      } or
+      MkModuleImport(string m) {
+        imports(_, m)
+        or
+        m = any(CanonicalName n | isUsed(n)).getExternalModuleName()
       } or
-      MkModuleImport(string m) { imports(_, m) } or
       MkClassInstance(DataFlow::ClassNode cls) { cls = trackDefNode(_) and hasSemantics(cls) } or
       MkAsyncFuncResult(DataFlow::FunctionNode f) {
         f = trackDefNode(_) and f.getFunction().isAsync() and hasSemantics(f)
       } or
       MkDef(DataFlow::Node nd) { rhs(_, _, nd) } or
       MkUse(DataFlow::Node nd) { use(_, _, nd) } or
-      MkCanonicalNameDef(CanonicalName n) { isDefined(n) } or
-      MkCanonicalNameUse(CanonicalName n) { isUsed(n) }
+      /**
+       * A TypeScript canonical name that is defined somewhere, and that isn't a module root.
+       * (Module roots are represented by `MkModuleExport` nodes instead.)
+       *
+       * For most purposes, you probably want to use the `mkCanonicalNameDef` predicate instead of
+       * this constructor.
+       */
+      MkCanonicalNameDef(CanonicalName n) {
+        not n.isRoot() and
+        isDefined(n)
+      } or
+      /**
+       * A TypeScript canonical name that is used somewhere, and that isn't a module root.
+       * (Module roots are represented by `MkModuleImport` nodes instead.)
+       *
+       * For most purposes, you probably want to use the `mkCanonicalNameUse` predicate instead of
+       * this constructor.
+       */
+      MkCanonicalNameUse(CanonicalName n) {
+        not n.isRoot() and
+        isUsed(n)
+      }
 
     class TDef = MkModuleDef or TNonModuleDef;
 
@@ -399,6 +433,20 @@ module API {
       )
     }
 
+    /** An API-graph node representing definitions of the canonical name `cn`. */
+    private TApiNode mkCanonicalNameDef(CanonicalName cn) {
+      if cn.isModuleRoot()
+      then result = MkModuleExport(cn.getExternalModuleName())
+      else result = MkCanonicalNameDef(cn)
+    }
+
+    /** An API-graph node representing uses of the canonical name `cn`. */
+    private TApiNode mkCanonicalNameUse(CanonicalName cn) {
+      if cn.isModuleRoot()
+      then result = MkModuleImport(cn.getExternalModuleName())
+      else result = MkCanonicalNameUse(cn)
+    }
+
     /**
      * Holds if `rhs` is the right-hand side of a definition of a node that should have an
      * incoming edge from `base` labeled `lbl` in the API graph.
@@ -713,20 +761,11 @@ module API {
         succ = MkClassInstance(trackDefNode(def))
       )
       or
-      exists(CanonicalName cn |
-        pred = MkRoot() and
-        lbl = Label::mod(cn.getExternalModuleName())
-      |
-        succ = MkCanonicalNameUse(cn) or
-        succ = MkCanonicalNameDef(cn)
-      )
-      or
-      exists(CanonicalName cn1, CanonicalName cn2 |
-        cn2 = cn1.getAChild() and
-        lbl = Label::member(cn2.getName())
-      |
-        (pred = MkCanonicalNameDef(cn1) or pred = MkCanonicalNameUse(cn1)) and
-        (succ = MkCanonicalNameDef(cn2) or succ = MkCanonicalNameUse(cn2))
+      exists(CanonicalName cn1, string n, CanonicalName cn2 |
+        pred in [mkCanonicalNameDef(cn1), mkCanonicalNameUse(cn1)] and
+        cn2 = cn1.getChild(n) and
+        lbl = Label::member(n) and
+        succ in [mkCanonicalNameDef(cn2), mkCanonicalNameUse(cn2)]
       )
       or
       exists(DataFlow::Node nd, DataFlow::FunctionNode f |

javascript/ql/src/semmle/javascript/CanonicalNames.qll

@@ -25,10 +25,18 @@ class CanonicalName extends @symbol {
   CanonicalName getParent() { symbol_parent(this, result) }
 
   /**
-   * Gets a child of this canonical name, i.e. an extension of its qualified name.
+   * Gets a child of this canonical name, that is, an extension of its qualified name.
    */
   CanonicalName getAChild() { result.getParent() = this }
 
+  /**
+   * Gets the child of this canonical name that has the given `name`, if any.
+   */
+  CanonicalName getChild(string name) {
+    result = getAChild() and
+    result.getName() = name
+  }
+
   /**
    * Gets the name without prefix.
    */

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

@@ -48,20 +48,6 @@ private module MongoDB {
     not result.getAnImmediateUse().(DataFlow::ParameterNode).getName() = "client" // mongodb v3 provides a `Mongoclient` here
   }
 
-  /**
-   * A collection based on the type `mongodb.Collection`.
-   *
-   * Note that this also covers `mongoose` models since they are subtypes
-   * of `mongodb.Collection`.
-   */
-  private class TypedMongoCollection extends API::EntryPoint {
-    TypedMongoCollection() { this = "TypedMongoCollection" }
-
-    override DataFlow::SourceNode getAUse() { result.hasUnderlyingType("mongodb", "Collection") }
-
-    override DataFlow::Node getARhs() { none() }
-  }
-
   /** Gets a data flow node referring to a MongoDB collection. */
   private API::Node getACollection() {
     // A collection resulting from calling `Db.collection(...)`.
@@ -71,7 +57,8 @@ private module MongoDB {
       result = collection.getParameter(1).getParameter(0)
     )
     or
-    result = any(TypedMongoCollection c).getNode()
+    // note that this also covers `mongoose` models since they are subtypes of `mongodb.Collection`
+    result = API::Node::ofType("mongodb", "Collection")
   }
 
   /** A call to a MongoDB query method. */
@@ -225,17 +212,6 @@ private module Mongoose {
       }
     }
 
-    /**
-     * A Mongoose collection based on the type `mongoose.Model`.
-     */
-    private class TypedMongooseModel extends API::EntryPoint {
-      TypedMongooseModel() { this = "TypedMongooseModel" }
-
-      override DataFlow::SourceNode getAUse() { result.hasUnderlyingType("mongoose", "Model") }
-
-      override DataFlow::Node getARhs() { none() }
-    }
-
     /**
      * Gets a API-graph node referring to a Mongoose Model object.
      */
@@ -247,7 +223,7 @@ private module Mongoose {
         result = conn.getMember("models").getAMember()
       )
       or
-      result = any(TypedMongooseModel c).getNode()
+      result = API::Node::ofType("mongoose", "Model")
     }
 
     /**
@@ -341,24 +317,13 @@ private module Mongoose {
       override API::Node getQueryArgument() { result = this.getParameter(2) }
     }
 
-    /**
-     * A Mongoose query.
-     */
-    private class TypedMongooseQuery extends API::EntryPoint {
-      TypedMongooseQuery() { this = "TypedMongooseQuery" }
-
-      override DataFlow::SourceNode getAUse() { result.hasUnderlyingType("mongoose", "Query") }
-
-      override DataFlow::Node getARhs() { none() }
-    }
-
     /**
      * Gets a data flow node referring to a Mongoose query object.
      */
     API::Node getAMongooseQuery() {
       result = any(MongooseFunction f).getQueryReturn()
       or
-      result = any(TypedMongooseQuery c).getNode()
+      result = API::Node::ofType("mongoose", "Query")
       or
       result =
         getAMongooseQuery()
@@ -560,23 +525,14 @@ private module Mongoose {
       }
     }
 
-    /**
-     * A Mongoose document.
-     */
-    private class TypedMongooseDocument extends API::EntryPoint {
-      TypedMongooseDocument() { this = "TypedMongooseDocument" }
-
-      override DataFlow::SourceNode getAUse() { result.hasUnderlyingType("mongoose", "Document") }
-
-      override DataFlow::Node getARhs() { none() }
-    }
-
     /**
      * Gets a data flow node referring to a Mongoose Document object.
      */
     private API::Node getAMongooseDocument() {
-      result instanceof RetrievedDocument or
-      result = any(TypedMongooseDocument c).getNode() or
+      result instanceof RetrievedDocument
+      or
+      result = API::Node::ofType("mongoose", "Document")
+      or
       result =
         getAMongooseDocument()
             .getMember(any(string name | MethodSignatures::returnsDocument(name)))

javascript/ql/test/ApiGraphs/typed/NodeOfType.expected

@@ -0,0 +1,3 @@
+| mongodb | Collection | index.ts:14:3:14:17 | getCollection() |
+| mongoose | Model | index.ts:22:3:22:20 | getMongooseModel() |
+| mongoose | Query | index.ts:23:3:23:20 | getMongooseQuery() |

javascript/ql/test/ApiGraphs/typed/NodeOfType.ql

@@ -0,0 +1,4 @@
+import javascript
+
+from string mod, string tp
+select mod, tp, API::Node::ofType(mod, tp).getAnImmediateUse()

javascript/ql/test/ApiGraphs/typed/index.ts

@@ -11,14 +11,14 @@ app.use(bodyParser.json());
 
 app.post("/find", (req, res) => {
   let v = JSON.parse(req.body.x);
-  getCollection().find({ id: v }); /* use (member find (instance (member Collection (module mongodb)))) */
+  getCollection().find({ id: v }); /* use (member find (instance (member Collection (member exports (module mongodb))))) */
 });
 
 import * as mongoose from "mongoose";
 declare function getMongooseModel(): mongoose.Model;
 declare function getMongooseQuery(): mongoose.Query;
 app.post("/find", (req, res) => {
   let v = JSON.parse(req.body.x);
-  getMongooseModel().find({ id: v }); /* def (parameter 0 (member find (instance (member Model (module mongoose))))) */
-  getMongooseQuery().find({ id: v }); /* def (parameter 0 (member find (instance (member Query (module mongoose))))) */
+  getMongooseModel().find({ id: v }); /* def (parameter 0 (member find (instance (member Model (member exports (module mongoose)))))) */
+  getMongooseQuery().find({ id: v }); /* def (parameter 0 (member find (instance (member Query (member exports (module mongoose)))))) */
 });