JS: make moduleImport() work for named imports

Author: asger-semmle

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

@@ -37,6 +37,9 @@ module DataFlow {
     TThisNode(StmtContainer f) { f.(Function).getThisBinder() = f or f instanceof TopLevel } or
     TUnusedParameterNode(SimpleParameter p) {
       not exists(SsaExplicitDefinition ssa | p = ssa.getDef())
+    } or
+    TDestructuredModuleImportNode(ImportDeclaration decl) {
+      decl.getASpecifier() instanceof NamedImportSpecifier
     }
 
   /**
@@ -342,6 +345,28 @@ module DataFlow {
     override string toString() { result = "reflective call" }
   }
 
+  /**
+   * A node referring to the module imported at a named ES2015 import declaration.
+   *
+   * Default imports and namespace imports do not fall into this category, as the
+   * SSA definition of the local variable is used as the source of the module instead.
+   */
+  private class DestructuredModuleImportNode extends Node, TDestructuredModuleImportNode {
+    ImportDeclaration imprt;
+
+    DestructuredModuleImportNode() { this = TDestructuredModuleImportNode(imprt) }
+
+    override BasicBlock getBasicBlock() { result = imprt.getBasicBlock() }
+
+    override predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      imprt.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    }
+
+    override string toString() { result = imprt.toString() }
+  }
+
   /**
    * A data flow node that reads or writes an object property or class member.
    *
@@ -372,6 +397,12 @@ module DataFlow {
      */
     abstract string getPropertyName();
 
+    /**
+     * Holds if this creates an alias for the property, as opposed to
+     * just being a read or write of the property.
+     */
+    predicate isES2015PropertyBinding() { none() }
+
     /**
      * Holds if this data flow node accesses property `p` on base node `base`.
      */
@@ -659,6 +690,31 @@ module DataFlow {
     override string getPropertyName() { none() }
   }
 
+  /**
+   * A named import specifier seen as a property read on the imported module.
+   */
+  private class NamedImportSpecifierAsPropRead extends PropRead {
+    ImportDeclaration imprt;
+
+    NamedImportSpecifier spec;
+
+    NamedImportSpecifierAsPropRead() {
+      spec = imprt.getASpecifier() and
+      exists(SsaExplicitDefinition ssa |
+        ssa.getDef() = spec and
+        this = TSsaDefNode(ssa)
+      )
+    }
+
+    override Node getBase() { result = TDestructuredModuleImportNode(imprt) }
+
+    override Expr getPropertyNameExpr() { result = spec.getImported() }
+
+    override string getPropertyName() { result = spec.getImportedName() }
+
+    override predicate isES2015PropertyBinding() { any() }
+  }
+
   /**
    * A data flow node representing an unused parameter.
    *
@@ -875,6 +931,16 @@ module DataFlow {
    */
   DataFlow::ThisNode thisNode(StmtContainer container) { result = TThisNode(container) }
 
+  /**
+   * INTERNAL. DO NOT USE.
+   *
+   * Gets the data flow node holding the reference to the module being destructured at
+   * the given import declaration.
+   */
+  DataFlow::Node destructuredModuleImportNode(ImportDeclaration imprt) {
+    result = TDestructuredModuleImportNode(imprt)
+  }
+
   /**
    * A classification of flows that are not modeled, or only modeled incompletely, by
    * `DataFlowNode`:

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

@@ -427,6 +427,12 @@ class ModuleImportNode extends DataFlow::SourceNode {
       is.getImportedName() = "default"
     )
     or
+    // `import { createServer } from 'http'`
+    exists(ImportDeclaration id |
+      this = DataFlow::destructuredModuleImportNode(id) and
+      id.getImportedPath().getValue() = path
+    )
+    or
     // declared AMD dependency
     exists(AMDModuleDefinition amd |
       this = DataFlow::parameterNode(amd.getDependencyParameter(path))
@@ -456,14 +462,6 @@ ModuleImportNode moduleImport(string path) { result.getPath() = path }
  */
 DataFlow::SourceNode moduleMember(string path, string m) {
   result = moduleImport(path).getAPropertyRead(m)
-  or
-  exists(ImportDeclaration id, ImportSpecifier is, SsaExplicitDefinition ssa |
-    id.getImportedPath().getValue() = path and
-    is = id.getASpecifier() and
-    is.getImportedName() = m and
-    ssa.getDef() = is and
-    result = DataFlow::ssaDefinitionNode(ssa)
-  )
 }
 
 /**

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

@@ -207,6 +207,8 @@ module SourceNode {
       this instanceof DataFlow::Impl::InvokeNodeDef
       or
       DataFlow::thisNode(this, _)
+      or
+      this = DataFlow::destructuredModuleImportNode(_)
     }
   }
 }

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

@@ -175,6 +175,25 @@ private class AnalyzedNamespaceImport extends AnalyzedImport {
   }
 }
 
+/**
+ * Flow analysis for namespace imports.
+ */
+private class AnalyzedDestructuredImport extends AnalyzedPropertyRead {
+  Module imported;
+
+  AnalyzedDestructuredImport() {
+    exists(ImportDeclaration id |
+      this = DataFlow::destructuredModuleImportNode(id) and
+      imported = id.getImportedModule()
+    )
+  }
+
+  override predicate reads(AbstractValue base, string propName) {
+    base = TAbstractModuleObject(imported) and
+    propName = "exports"
+  }
+}
+
 /**
  * Flow analysis for `require` calls, interpreted as an implicit read of
  * the `module.exports` property of the imported module.

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

@@ -12,6 +12,7 @@
 | sources.js:3:2:5:1 | functio ... x+19;\\n} |
 | sources.js:3:11:3:11 | x |
 | tst.js:1:1:1:0 | this |
+| tst.js:1:1:1:24 | import  ... m 'fs'; |
 | tst.js:1:10:1:11 | fs |
 | tst.js:16:1:20:9 | (functi ... ("arg") |
 | tst.js:16:2:16:1 | this |

javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getAConstructorInvocation.expected

@@ -1,2 +1,3 @@
+| destructuringES6.js:1:1:1:41 | import  ... ctron'; | destructuringES6.js:2:1:2:19 | new BrowserWindow() |
 | destructuringRequire.js:1:27:1:45 | require('electron') | destructuringRequire.js:2:1:2:19 | new BrowserWindow() |
 | moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:9:1:9:7 | new K() |

javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getAMemberInvocation.expected

@@ -1,5 +1,6 @@
 | amd1.js:1:25:1:26 | fs | amd1.js:2:3:2:29 | fs.read ... a.txt") |
 | amd2.js:2:12:2:24 | require('fs') | amd2.js:3:3:3:29 | fs.read ... a.txt") |
+| destructuringES6.js:1:1:1:41 | import  ... ctron'; | destructuringES6.js:2:1:2:19 | new BrowserWindow() |
 | destructuringRequire.js:1:27:1:45 | require('electron') | destructuringRequire.js:2:1:2:19 | new BrowserWindow() |
 | moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:3:1:3:18 | mod.moduleMethod() |
 | moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:6:1:6:3 | f() |

javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getAPropertyRead.expected

@@ -1,5 +1,6 @@
 | amd1.js:1:25:1:26 | fs | amd1.js:2:3:2:17 | fs.readFileSync |
 | amd2.js:2:12:2:24 | require('fs') | amd2.js:3:3:3:17 | fs.readFileSync |
+| destructuringES6.js:1:1:1:41 | import  ... ctron'; | destructuringES6.js:1:10:1:22 | BrowserWindow |
 | destructuringRequire.js:1:27:1:45 | require('electron') | destructuringRequire.js:1:9:1:21 | BrowserWindow |
 | moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:3:1:3:16 | mod.moduleMethod |
 | moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:5:9:5:26 | mod.moduleFunction |

javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getPath.expected

@@ -1,5 +1,6 @@
 | amd1.js:1:25:1:26 | fs | fs |
 | amd2.js:2:12:2:24 | require('fs') | fs |
+| destructuringES6.js:1:1:1:41 | import  ... ctron'; | electron |
 | destructuringRequire.js:1:27:1:45 | require('electron') | electron |
 | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName | myDefaultImportedModuleInstance |
 | instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName | myNamespaceImportedModuleInstance |

javascript/ql/test/library-tests/ModuleImportNodes/moduleImport.expected

@@ -0,0 +1,8 @@
+| electron | destructuringES6.js:1:1:1:41 | import  ... ctron'; |
+| electron | destructuringRequire.js:1:27:1:45 | require('electron') |
+| fs | amd1.js:1:25:1:26 | fs |
+| fs | amd2.js:2:12:2:24 | require('fs') |
+| mod | moduleUses.js:1:11:1:24 | require('mod') |
+| myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName |
+| myNamespaceImportedModuleInstance | instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName |
+| myRequiredModuleInstance | instanceThroughRequire.js:1:36:1:70 | require ... tance') |