diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 3bb04f2686be..8f0a9047498d 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/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.getImportedModuleNodeIfUnambiguous() and m = imp.getImportedModule() and lbl = Label::member(prop) and rhs = m.getAnExportedValue(prop) @@ -1337,7 +1337,9 @@ 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.getImportedModuleNodeIfUnambiguous() = trackDefNode(nd, t.continue()) + | result = DataFlow::exportsVarNode(imp.getImportedModule()) or result = DataFlow::moduleVarNode(imp.getImportedModule()).getAPropertyRead("exports") diff --git a/javascript/ql/lib/semmle/javascript/ES2015Modules.qll b/javascript/ql/lib/semmle/javascript/ES2015Modules.qll index e7534449f55b..5dbd91ef980b 100644 --- a/javascript/ql/lib/semmle/javascript/ES2015Modules.qll +++ b/javascript/ql/lib/semmle/javascript/ES2015Modules.qll @@ -130,18 +130,18 @@ class ImportDeclaration extends Stmt, Import, @import_declaration { override DataFlow::Node getImportedModuleNode() { // `import * as http from 'http'` or `import http from `http`' + not exists(DataFlow::destructuredModuleImportNode(this)) and exists(ImportSpecifier is | is = this.getASpecifier() and result = DataFlow::valueNode(is) | - is instanceof ImportNamespaceSpecifier and - count(this.getASpecifier()) = 1 + is instanceof ImportNamespaceSpecifier 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" + // treat default imports as namespace imports. In cases where it causes ambiguity + // between named exports and the properties of a default-exported object, the caller + // of `getImportedModuleNode()` must check `isDefaultImport()` and correct the behavior. + this.hasOnlyDefaultImport() ) or // `import { createServer } from 'http'` @@ -152,6 +152,12 @@ class ImportDeclaration extends Stmt, Import, @import_declaration { predicate isTypeOnly() { has_type_keyword(this) } override string getAPrimaryQlClass() { result = "ImportDeclaration" } + + private predicate hasOnlyDefaultImport() { + unique( | | this.getASpecifier()) instanceof ImportDefaultSpecifier + } + + override predicate isDefaultImport() { this.hasOnlyDefaultImport() } } /** A literal path expression appearing in an `import` declaration. */ diff --git a/javascript/ql/lib/semmle/javascript/Modules.qll b/javascript/ql/lib/semmle/javascript/Modules.qll index 3ddd5132d05d..e4b8941e98c6 100644 --- a/javascript/ql/lib/semmle/javascript/Modules.qll +++ b/javascript/ql/lib/semmle/javascript/Modules.qll @@ -179,7 +179,58 @@ 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 referring to imported module object. + * + * For example: + * ```js + * // ES2015 style + * import * as foo from "bar"; // gets the node for `foo` + * import foo from "bar"; // gets the node for `foo` (see note on default imports below) + * + * // CommonJS style + * require("bar"); // gets the node for the `require` call + * + * // AMD style + * define(["bar"], function(foo) { // gets the node for the `foo` parameter + * }) + * ``` + * + * For statements of form `import foo from "bar'`, this gives the node corresponding to `foo`. + * Technically this should refer to the export binding named `"default"`, not the whole module, but for compatibility with non-standard + * interpretations of default imports, this node is usually treated as also referring to the whole module. + * If this behavior is not wanted, use `isDefaultImport()` to handle that case differently. */ abstract DataFlow::Node getImportedModuleNode(); + + /** + * Holds if the result of `getImportedModuleNode` actually refers to the export binding named `"default"`, + * as opposed an object whose properties correspond to the export bindings of the imported module. + * + * For compatibility with non-standard interpretations of `default` imports, the default + * import is usually returned by `getImportedModuleNode()`. If such behavior is not wanted, + * this predicate can be used to handle that case differently. + * + * For example, `getImportedModuleNode()` returns `foo` in both of these imports, but `isDefaultImport()` + * only holds for the first one: + * ```js + * import foo from "bar"; + * import * as foo from "bar"; + * ``` + */ + predicate isDefaultImport() { none() } + + /** + * Gets the same as `getImportedModuleNode()` except this has no result for default imports when the target module + * has both default and named exports. + * + * This is to avoid ambiguity between named export bindings and the properties of the default-exported object. + */ + pragma[nomagic] + final DataFlow::Node getImportedModuleNodeIfUnambiguous() { + result = this.getImportedModuleNode() and + not ( + this.isDefaultImport() and + this.getImportedModule().(ES2015Module).hasBothNamedAndDefaultExports() + ) + } }