Merge pull request #934 from asger-semmle/module-import

semmle-qlci · web-flow · commit 014d4b9ed0be · 2019-02-25T09:46:52.000Z
Approved by xiemaisi
diff --git a/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll b/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) {
+      exists(decl.getASpecifier().getImportedName())
     }
 
   /**
@@ -351,6 +354,25 @@ module DataFlow {
     override string toString() { result = "reflective call" }
   }
 
+  /**
+   * A node referring to the module imported at a named or default ES2015 import declaration.
+   */
+  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.
    *
@@ -668,6 +690,30 @@ module DataFlow {
     override string getPropertyName() { none() }
   }
 
+  /**
+   * A named import specifier seen as a property read on the imported module.
+   */
+  private class ImportSpecifierAsPropRead extends PropRead {
+    ImportDeclaration imprt;
+
+    ImportSpecifier spec;
+
+    ImportSpecifierAsPropRead() {
+      spec = imprt.getASpecifier() and
+      exists(spec.getImportedName()) 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() }
+  }
+
   /**
    * A data flow node representing an unused parameter.
    *
@@ -886,6 +932,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`:
diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
@@ -429,6 +429,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))
@@ -458,14 +464,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)
-  )
 }
 
 /**
diff --git a/javascript/ql/src/semmle/javascript/dataflow/Sources.qll b/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(_)
     }
   }
 }
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll b/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.
diff --git a/javascript/ql/test/library-tests/DataFlow/sources.expected b/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 |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
@@ -18,6 +18,7 @@
 | global.js:5:22:5:35 | "also tainted" | global.js:9:13:9:22 | g(source1) |
 | global.js:5:22:5:35 | "also tainted" | global.js:10:13:10:22 | g(source2) |
 | nodeJsLib.js:1:15:1:23 | "tainted" | esClient.js:7:13:7:18 | nj.foo |
+| nodeJsLib.js:1:15:1:23 | "tainted" | esClient.js:10:13:10:17 | njFoo |
 | nodeJsLib.js:1:15:1:23 | "tainted" | nodeJsClient.js:4:13:4:18 | nj.foo |
 | nodeJsLib.js:2:15:2:23 | "tainted" | esClient.js:7:13:7:18 | nj.foo |
 | nodeJsLib.js:2:15:2:23 | "tainted" | esClient.js:10:13:10:17 | njFoo |
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getAConstructorInvocation.expected b/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() |
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getAMemberInvocation.expected b/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() |
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getAPropertyRead.expected b/javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getAPropertyRead.expected
@@ -1,6 +1,8 @@
 | 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 |
+| instanceThroughDefaultImport.js:1:1:1:82 | import  ... tance'; | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName |
 | 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 |
 | moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:8:9:8:31 | mod.con ... unction |
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getPath.expected b/javascript/ql/test/library-tests/ModuleImportNodes/ModuleImportNode_getPath.expected
@@ -1,6 +1,8 @@
 | 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:1:1:82 | import  ... tance'; | myDefaultImportedModuleInstance |
 | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName | myDefaultImportedModuleInstance |
 | instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName | myNamespaceImportedModuleInstance |
 | instanceThroughRequire.js:1:36:1:70 | require ... tance') | myRequiredModuleInstance |
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/moduleImport.expected b/javascript/ql/test/library-tests/ModuleImportNodes/moduleImport.expected
@@ -0,0 +1,9 @@
+| 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:1:1:82 | import  ... tance'; |
+| 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') |
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/moduleImport.ql b/javascript/ql/test/library-tests/ModuleImportNodes/moduleImport.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from string path
+select path, DataFlow::moduleImport(path)
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/moduleImportProp.expected b/javascript/ql/test/library-tests/ModuleImportNodes/moduleImportProp.expected
@@ -0,0 +1,9 @@
+| electron | BrowserWindow | destructuringES6.js:1:10:1:22 | BrowserWindow |
+| electron | BrowserWindow | destructuringRequire.js:1:9:1:21 | BrowserWindow |
+| fs | readFileSync | amd1.js:2:3:2:17 | fs.readFileSync |
+| fs | readFileSync | amd2.js:3:3:3:17 | fs.readFileSync |
+| mod | constructorFunction | moduleUses.js:8:9:8:31 | mod.con ... unction |
+| mod | moduleField | moduleUses.js:11:1:11:15 | mod.moduleField |
+| mod | moduleFunction | moduleUses.js:5:9:5:26 | mod.moduleFunction |
+| mod | moduleMethod | moduleUses.js:3:1:3:16 | mod.moduleMethod |
+| myDefaultImportedModuleInstance | default | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName |
diff --git a/javascript/ql/test/library-tests/ModuleImportNodes/moduleImportProp.ql b/javascript/ql/test/library-tests/ModuleImportNodes/moduleImportProp.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from string path, string prop
+select path, prop, DataFlow::moduleImport(path).getAPropertyRead(prop)
diff --git a/javascript/ql/test/library-tests/Portals/PortalExit.expected b/javascript/ql/test/library-tests/Portals/PortalExit.expected
@@ -1038,4 +1038,6 @@
 | (return (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
 | (return (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
 | (root https://www.npmjs.com/package/m1) | src/m3/index.js:1:10:1:22 | require("m1") | false |
+| (root https://www.npmjs.com/package/m2) | src/m3/tst2.js:1:1:1:25 | import  ... m "m2"; | false |
+| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:1:1:19 | import A from "m2"; | false |
 | (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:8:1:8 | A | false |

Original file line number	Diff line number	Diff line change
`@@ -207,6 +207,8 @@ module SourceNode {`
`207`	`207`	`this instanceof DataFlow::Impl::InvokeNodeDef`
`208`	`208`	`or`
`209`	`209`	`DataFlow::thisNode(this, _)`
	`210`	`+ or`
	`211`	`+ this = DataFlow::destructuredModuleImportNode(_)`
`210`	`212`	`}`
`211`	`213`	`}`
`212`	`214`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
	`1`	`+\| destructuringES6.js:1:1:1:41 \| import ... ctron'; \| destructuringES6.js:2:1:2:19 \| new BrowserWindow() \|`
`1`	`2`	`\| destructuringRequire.js:1:27:1:45 \| require('electron') \| destructuringRequire.js:2:1:2:19 \| new BrowserWindow() \|`
`2`	`3`	`\| moduleUses.js:1:11:1:24 \| require('mod') \| moduleUses.js:9:1:9:7 \| new K() \|`