Merge pull request #4247 from erik-krogh/CVE760-reexport

Approved by asgerf

javascript/ql/src/NodeJS/MissingExports.ql

@@ -20,9 +20,9 @@ predicate definedInModule(GlobalVariable v, NodeModule m) {
   )
 }
 
-from NodeModule m, GlobalVariable f, InvokeExpr invk, ASTNode export, GlobalVarAccess acc
+from NodeModule m, GlobalVariable f, InvokeExpr invk, DataFlow::Node export, GlobalVarAccess acc
 where
-  m.exports(f.getName(), export) and
+  export = m.getAnExportedValue(f.getName()) and
   acc = f.getAnAccess() and
   invk.getCallee() = acc and
   invk.getTopLevel() = m and

javascript/ql/src/semmle/javascript/AMD.qll

@@ -295,8 +295,8 @@ class AmdModule extends Module {
   /** Gets the definition of this module. */
   AmdModuleDefinition getDefine() { amdModuleTopLevel(result, this) }
 
-  override predicate exports(string name, ASTNode export) {
-    exists(DataFlow::PropWrite pwn | export = pwn.getAstNode() |
+  override DataFlow::Node getAnExportedValue(string name) {
+    exists(DataFlow::PropWrite pwn | result = pwn.getRhs() |
       pwn.getBase().analyze().getAValue() = getDefine().getAModuleExportsValue() and
       name = pwn.getPropertyName()
     )

javascript/ql/src/semmle/javascript/Closure.qll

@@ -165,9 +165,9 @@ module Closure {
       result = getScope().getVariable("exports")
     }
 
-    override predicate exports(string name, ASTNode export) {
+    override DataFlow::Node getAnExportedValue(string name) {
       exists(DataFlow::PropWrite write, Expr base |
-        write.getAstNode() = export and
+        result = write.getRhs() and
         write.writes(base.flow(), name, _) and
         (
           base = getExportsVariable().getAReference()

javascript/ql/src/semmle/javascript/ES2015Modules.qll

@@ -27,8 +27,8 @@ class ES2015Module extends Module {
   /** Gets an export declaration in this module. */
   ExportDeclaration getAnExport() { result.getTopLevel() = this }
 
-  override predicate exports(string name, ASTNode export) {
-    exists(ExportDeclaration ed | ed = getAnExport() and ed = export | ed.exportsAs(_, name))
+  override DataFlow::Node getAnExportedValue(string name) {
+    exists(ExportDeclaration ed | ed = getAnExport() and result = ed.getSourceNode(name))
   }
 
   /** Holds if this module exports variable `v` under the name `name`. */
@@ -395,6 +395,13 @@ class ExportNamedDeclaration extends ExportDeclaration, @export_named_declaratio
       result = DataFlow::valueNode(d.getSource())
     )
     or
+    exists(ObjectPattern obj | obj = getOperand().(DeclStmt).getADecl().getBindingPattern() |
+      exists(DataFlow::PropRead read | read = result |
+        read.getBase() = obj.flow() and
+        name = read.getPropertyName()
+      )
+    )
+    or
     exists(ExportSpecifier spec | spec = getASpecifier() and name = spec.getExportedName() |
       not exists(getImportedPath()) and result = DataFlow::valueNode(spec.getLocal())
       or

javascript/ql/src/semmle/javascript/Modules.qll

@@ -24,9 +24,11 @@ abstract class Module extends TopLevel {
   Module getAnImportedModule() { result = getAnImport().getImportedModule() }
 
   /** Gets a symbol exported by this module. */
-  string getAnExportedSymbol() { exports(result, _) }
+  string getAnExportedSymbol() { exists(getAnExportedValue(result)) }
 
   /**
+   * DEPRECATED. Use `getAnExportedValue` instead.
+   *
    * Holds if this module explicitly exports symbol `name` at the
    * program element `export`.
    *
@@ -36,9 +38,77 @@ abstract class Module extends TopLevel {
    * that are explicitly defined on the module object.
    *
    * Symbols defined in another module that are re-exported by
-   * this module are not considered either.
+   * this module are only sometimes considered.
    */
-  abstract predicate exports(string name, ASTNode export);
+  deprecated predicate exports(string name, ASTNode export) {
+    this instanceof AmdModule and
+    exists(DataFlow::PropWrite pwn | export = pwn.getAstNode() |
+      pwn.getBase().analyze().getAValue() = this.(AmdModule).getDefine().getAModuleExportsValue() and
+      name = pwn.getPropertyName()
+    )
+    or
+    this instanceof Closure::ClosureModule and
+    exists(DataFlow::PropWrite write, Expr base |
+      write.getAstNode() = export and
+      write.writes(base.flow(), name, _) and
+      (
+        base = this.(Closure::ClosureModule).getExportsVariable().getAReference()
+        or
+        base = this.(Closure::ClosureModule).getExportsVariable().getAnAssignedExpr()
+      )
+    )
+    or
+    this instanceof NodeModule and
+    (
+      // a property write whose base is `exports` or `module.exports`
+      exists(DataFlow::PropWrite pwn | export = pwn.getAstNode() |
+        pwn.getBase() = this.(NodeModule).getAModuleExportsNode() and
+        name = pwn.getPropertyName()
+      )
+      or
+      // a re-export using spread-operator. E.g. `const foo = require("./foo"); module.exports = {bar: bar, ...foo};`
+      exists(ObjectExpr obj | obj = this.(NodeModule).getAModuleExportsNode().asExpr() |
+        obj
+            .getAProperty()
+            .(SpreadProperty)
+            .getInit()
+            .(SpreadElement)
+            .getOperand()
+            .flow()
+            .getALocalSource()
+            .asExpr()
+            .(Import)
+            .getImportedModule()
+            .exports(name, export)
+      )
+      or
+      // an externs definition (where appropriate)
+      exists(PropAccess pacc | export = pacc |
+        pacc.getBase() = this.(NodeModule).getAModuleExportsNode().asExpr() and
+        name = pacc.getPropertyName() and
+        isExterns() and
+        exists(pacc.getDocumentation())
+      )
+    )
+    or
+    this instanceof ES2015Module and
+    exists(ExportDeclaration ed | ed = this.(ES2015Module).getAnExport() and ed = export |
+      ed.exportsAs(_, name)
+    )
+  }
+
+  /**
+   * Get a value that is explicitly exported from this module with under `name`.
+   *
+   * Note that in some module systems (notably CommonJS and AMD)
+   * modules are arbitrary objects that export all their
+   * properties. This predicate only considers properties
+   * that are explicitly defined on the module object.
+   *
+   * Symbols defined in another module that are re-exported by
+   * this module are only sometimes considered.
+   */
+  abstract DataFlow::Node getAnExportedValue(string name);
 
   /**
    * Gets the root folder relative to which the given import path (which must

javascript/ql/src/semmle/javascript/NodeJS.qll

@@ -42,22 +42,64 @@ class NodeModule extends Module {
     )
   }
 
-  /** Gets a symbol exported by this module. */
-  override string getAnExportedSymbol() {
-    result = super.getAnExportedSymbol() or
-    result = getAnImplicitlyExportedSymbol()
+  /**
+   * Gets an expression that is an alias for `module.exports`.
+   * For performance this predicate only computes relevant expressions.
+   * So if using this predicate - consider expanding the list of relevant expressions.
+   */
+  pragma[noinline]
+  DataFlow::Node getAModuleExportsNode() {
+    (
+      // A bit of manual magic
+      result = any(DataFlow::PropWrite w | exists(w.getPropertyName())).getBase()
+      or
+      result = DataFlow::valueNode(any(PropAccess p | exists(p.getPropertyName())).getBase())
+      or
+      result = DataFlow::valueNode(any(ObjectExpr obj))
+    ) and
+    result.analyze().getAValue() = getAModuleExportsValue()
   }
 
-  override predicate exports(string name, ASTNode export) {
+  /** Gets a symbol exported by this module. */
+  override string getAnExportedSymbol() {
+    result = super.getAnExportedSymbol()
+    or
+    result = getAnImplicitlyExportedSymbol()
+    or
+    // getters and the like.
+    exists(DataFlow::PropWrite pwn |
+      pwn.getBase() = this.getAModuleExportsNode() and
+      result = pwn.getPropertyName()
+    )
+  }
+
+  override DataFlow::Node getAnExportedValue(string name) {
     // a property write whose base is `exports` or `module.exports`
-    exists(DataFlow::PropWrite pwn | export = pwn.getAstNode() |
-      pwn.getBase().analyze().getAValue() = getAModuleExportsValue() and
+    exists(DataFlow::PropWrite pwn | result = pwn.getRhs() |
+      pwn.getBase() = getAModuleExportsNode() and
       name = pwn.getPropertyName()
     )
     or
+    // a re-export using spread-operator. E.g. `const foo = require("./foo"); module.exports = {bar: bar, ...foo};`
+    exists(ObjectExpr obj | obj = getAModuleExportsNode().asExpr() |
+      result =
+        obj
+            .getAProperty()
+            .(SpreadProperty)
+            .getInit()
+            .(SpreadElement)
+            .getOperand()
+            .flow()
+            .getALocalSource()
+            .asExpr()
+            .(Import)
+            .getImportedModule()
+            .getAnExportedValue(name)
+    )
+    or
     // an externs definition (where appropriate)
-    exists(PropAccess pacc | export = pacc |
-      pacc.getBase().analyze().getAValue() = getAModuleExportsValue() and
+    exists(PropAccess pacc | result = DataFlow::valueNode(pacc) |
+      pacc.getBase() = getAModuleExportsNode().asExpr() and
       name = pacc.getPropertyName() and
       isExterns() and
       exists(pacc.getDocumentation())

javascript/ql/src/semmle/javascript/PackageExports.qll

@@ -40,7 +40,7 @@ DataFlow::Node getAValueExportedBy(PackageJSON packageJSON) {
   exists(DataFlow::SourceNode callee |
     callee = getAValueExportedBy(packageJSON).(DataFlow::NewNode).getCalleeNode().getALocalSource()
   |
-    result = callee.getAPropertyRead("prototype").getAPropertyWrite()
+    result = callee.getAPropertyRead("prototype").getAPropertyWrite().getRhs()
     or
     result = callee.(DataFlow::ClassNode).getAnInstanceMethod()
   )
@@ -68,10 +68,5 @@ DataFlow::Node getAValueExportedBy(PackageJSON packageJSON) {
 private DataFlow::Node getAnExportFromModule(Module mod) {
   result.analyze().getAValue() = mod.(NodeModule).getAModuleExportsValue()
   or
-  exists(ASTNode export | result.getEnclosingExpr() = export | mod.exports(_, export))
-  or
-  exists(ExportDeclaration export |
-    result = export.getSourceNode(_) and
-    mod = export.getTopLevel()
-  )
+  result = mod.getAnExportedValue(_)
 }

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

@@ -233,7 +233,9 @@ private module NpmPackagePortal {
         apw.writes(m.(AnalyzedModule).getModuleObject(), "exports", exp)
       )
       or
-      m.(ES2015Module).exports("default", exp.(DataFlow::ValueNode).getAstNode())
+      exists(DataFlow::PropWrite export | exp = export |
+        export.getRhs() = m.(ES2015Module).getAnExportedValue("default")
+      )
     )
   }
 }

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

@@ -4,6 +4,7 @@
 
 private import javascript
 private import semmle.javascript.dataflow.internal.StepSummary
+private import semmle.javascript.dataflow.internal.PreCallGraphStep
 
 cached
 module CallGraph {
@@ -48,6 +49,10 @@ module CallGraph {
     t.start() and
     AccessPath::step(function, result)
     or
+    t.start() and
+    imprecision = 0 and
+    PreCallGraphStep::step(any(DataFlow::Node n | function.flowsTo(n)), result)
+    or
     imprecision = 0 and
     exists(DataFlow::ClassNode cls |
       exists(string name |

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

@@ -643,6 +643,20 @@ module NodeJSLib {
     }
   }
 
+  private import semmle.javascript.PackageExports as Exports
+
+  /**
+   * A direct step from an named export to a property-read reading the exported value.
+   */
+  private class ExportsStep extends PreCallGraphStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(Import imp, string name |
+        succ = DataFlow::valueNode(imp).(DataFlow::SourceNode).getAPropertyRead(name) and
+        pred = imp.getImportedModule().getAnExportedValue(name)
+      )
+    }
+  }
+
   /**
    * A call to a method from module `child_process`.
    */