JS: Refactor 'default' import interop

Author: asgerf

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -822,7 +822,7 @@ module API {
           or
           // special case: from `require('m')` to an export of `prop` in `m`
           exists(Import imp, Module m, string prop |
-            pred = imp.getImportedModuleNode() and
+            pred = imp.getImportedModuleNodeStrict() and
             m = imp.getImportedModule() and
             lbl = Label::member(prop) and
             rhs = m.getAnExportedValue(prop)
@@ -1337,7 +1337,7 @@ module API {
       result = nd.getALocalSource()
       or
       // additional backwards step from `require('m')` to `exports` or `module.exports` in m
-      exists(Import imp | imp.getImportedModuleNode() = trackDefNode(nd, t.continue()) |
+      exists(Import imp | imp.getImportedModuleNodeStrict() = trackDefNode(nd, t.continue()) |
         result = DataFlow::exportsVarNode(imp.getImportedModule())
         or
         result = DataFlow::moduleVarNode(imp.getImportedModule()).getAPropertyRead("exports")

javascript/ql/lib/semmle/javascript/ES2015Modules.qll

@@ -137,17 +137,26 @@ class ImportDeclaration extends Stmt, Import, @import_declaration {
       is instanceof ImportNamespaceSpecifier and
       count(this.getASpecifier()) = 1
       or
-      // For compatibility with the non-standard implementation of default imports,
-      // treat default imports as namespace imports in cases where it can't cause ambiguity
-      // between named exports and the properties of a default-exported object.
-      not this.getImportedModule().(ES2015Module).hasBothNamedAndDefaultExports() and
-      is.getImportedName() = "default"
+      result = this.getAmbiguousDefaultImportNode()
     )
     or
     // `import { createServer } from 'http'`
     result = DataFlow::destructuredModuleImportNode(this)
   }
 
+  /**
+   * Gets the data flow node corresponding to the `foo` in `import foo from "somewhere"`.
+   *
+   * This refers to the default import, but some non-standard compilers will treat it as a namespace
+   * import. In order to support both interpretations, it is considered an "ambiguous default import".
+   *
+   * Note that renamed default imports, such as `import { default as foo } from "somewhere"`,
+   * are not considered ambiguous, and will not be reported by this predicate.
+   */
+  DataFlow::Node getAmbiguousDefaultImportNode() {
+    result = DataFlow::valueNode(this.getASpecifier().(ImportDefaultSpecifier))
+  }
+
   /** Holds if this is declared with the `type` keyword, so it only imports types. */
   predicate isTypeOnly() { has_type_keyword(this) }
 

javascript/ql/lib/semmle/javascript/Modules.qll

@@ -179,7 +179,42 @@ abstract class Import extends AstNode {
   }
 
   /**
-   * Gets the data flow node that the default import of this import is available at.
+   * Gets the data flow node corresponding to the imported module.
+   *
+   * For example:
+   * ```js
+   * // ES2015 style
+   * import * as foo from "fs"; // Gets the node for `foo`
+   * import { readSync } from "fs"; // Gets a node representing the destructured import
+   *
+   * // CommonJS style
+   * require("fs"); // Gets the return value
+   *
+   * // AMD style
+   * define(["fs"], function(fs) { // Gets the node for the `fs` parameter
+   * });
+   * ```
+   *
+   * For default imports, this gets two nodes: the default import node, and a node representing the imported module:
+   * ```js
+   * import foo from "fs"; // gets both `foo` and a node representing the imported module
+   * ```
+   * This behaviour is to support non-standard compilers that treat default imports
+   * as namespace imports. Use `getImportedModuleNodeStrict()` to avoid this behaviour in cases
+   * where it would cause ambiguous data flow.
    */
   abstract DataFlow::Node getImportedModuleNode();
+
+  /**
+   * Gets the same as `getImportedModuleNode()` except ambiguous default imports are excluded
+   * in cases where it would cause ambiguity between named exports and properties
+   * of a default export.
+   */
+  final DataFlow::Node getImportedModuleNodeStrict() {
+    result = this.getImportedModuleNode() and
+    not (
+      result = this.(ImportDeclaration).getAmbiguousDefaultImportNode() and
+      this.getImportedModule().(ES2015Module).hasBothNamedAndDefaultExports()
+    )
+  }
 }