refactor package export into a library, and add tests for the library

Author: erik-krogh

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

@@ -0,0 +1,71 @@
+/**
+ * EXPERIMENTAL. This API may change in the future.
+ *
+ * Provides predicates for working with values exported from a package.
+ */
+
+import javascript
+
+/**
+ * Gets the number of occurrences of "/" in `path`.
+ */
+bindingset[path]
+private int countSlashes(string path) { result = count(path.splitAt("/")) - 1 }
+
+/**
+ * Gets the topmost package.json that appears in the project.
+ *
+ * There can be multiple results if the there exists multiple package.json that are equally deeply nested in the folder structure.
+ * Results are limited to package.json files that are at most nested 2 directories deep.
+ */
+PackageJSON getTopmostPackageJSON() {
+  result =
+    min(PackageJSON j |
+      countSlashes(j.getFile().getRelativePath()) <= 3
+    |
+      j order by countSlashes(j.getFile().getRelativePath())
+    )
+}
+
+/**
+ * Gets a value exported by the main module from the package.json `packageJSON`.
+ * The value is either directly the `module.exports` value, a nested property of `module.exports`, or a method on an exported class.
+ */
+DataFlow::Node getAValueExportedBy(PackageJSON packageJSON) {
+  result = getAnExportFromModule(packageJSON.getMainModule())
+  or
+  result = getAValueExportedBy(packageJSON).(DataFlow::PropWrite).getRhs()
+  or
+  exists(DataFlow::SourceNode callee |
+    callee = getAValueExportedBy(packageJSON).(DataFlow::NewNode).getCalleeNode().getALocalSource()
+  |
+    result = callee.getAPropertyRead("prototype").getAPropertyWrite()
+    or
+    result = callee.(DataFlow::ClassNode).getAnInstanceMethod()
+  )
+  or
+  result = getAValueExportedBy(packageJSON).getALocalSource()
+  or
+  result = getAValueExportedBy(packageJSON).(DataFlow::SourceNode).getAPropertyReference()
+  or
+  exists(Module mod |
+    mod = getAValueExportedBy(packageJSON).getEnclosingExpr().(Import).getImportedModule()
+  |
+    result = getAnExportFromModule(mod)
+  )
+  or
+  exists(DataFlow::ClassNode cla | cla = getAValueExportedBy(packageJSON) |
+    result = cla.getAnInstanceMethod() or
+    result = cla.getAStaticMethod() or
+    result = cla.getConstructor()
+  )
+}
+
+/**
+ * Gets an exported node from the module `mod`.
+ */
+private DataFlow::Node getAnExportFromModule(Module mod) {
+  result.analyze().getAValue() = mod.(NodeModule).getAModuleExportsValue()
+  or
+  exists(ASTNode export | result.getEnclosingExpr() = export | mod.exports(_, export))
+}

javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -5,7 +5,8 @@
  */
 
 import javascript
-import semmle.javascript.security.dataflow.RemoteFlowSources
+private import semmle.javascript.security.dataflow.RemoteFlowSources
+private import semmle.javascript.PackageExports as Exports
 
 /**
  * Module containing sources, sinks, and sanitizers for shell command constructed from library input.
@@ -44,76 +45,15 @@ module UnsafeShellCommandConstruction {
    */
   abstract class Sanitizer extends DataFlow::Node { }
 
-  /**
-   * Gets the number of occurrences of "/" in `path`.
-   */
-  bindingset[path]
-  private int countSlashes(string path) { result = count(path.splitAt("/")) - 1 }
-
-  /**
-   * Gets the topmost package.json that appears in the project.
-   *
-   * There can be multiple results if the there exists multiple package.json that are equally deeply nested in the folder structure.
-   * Results are limited to package.json files that are at most nested 2 directories deep.
-   */
-  private PackageJSON getTopmostPackageJSON() {
-    result =
-      min(PackageJSON j |
-        countSlashes(j.getFile().getRelativePath()) <= 3
-      |
-        j order by countSlashes(j.getFile().getRelativePath())
-      )
-  }
-
-  /**
-   * Gets a value exported by the main module from a package.json.
-   * The value is either directly the `module.exports` value, a nested property of `module.exports`, or a method on an exported class.
-   */
-  private DataFlow::Node getAnExportedValue() {
-    exists(PackageJSON pack | pack = getTopmostPackageJSON() |
-      result = getAnExportFromModule(pack.getMainModule())
-    )
-    or
-    result = getAnExportedValue().(DataFlow::PropWrite).getRhs()
-    or
-    exists(DataFlow::SourceNode callee |
-      callee = getAnExportedValue().(DataFlow::NewNode).getCalleeNode().getALocalSource()
-    |
-      result = callee.getAPropertyRead("prototype").getAPropertyWrite()
-      or
-      result = callee.(DataFlow::ClassNode).getAnInstanceMethod()
-    )
-    or
-    result = getAnExportedValue().getALocalSource()
-    or
-    result = getAnExportedValue().(DataFlow::SourceNode).getAPropertyReference()
-    or
-    exists(Module mod | mod = getAnExportedValue().getEnclosingExpr().(Import).getImportedModule() |
-      result = getAnExportFromModule(mod)
-    )
-    or
-    exists(DataFlow::ClassNode cla | cla = getAnExportedValue() |
-      result = cla.getAnInstanceMethod() or
-      result = cla.getAStaticMethod() or
-      result = cla.getConstructor()
-    )
-  }
-
-  /**
-   * Gets an exported node from the module `mod`.
-   */
-  private DataFlow::Node getAnExportFromModule(Module mod) {
-    result.analyze().getAValue() = mod.(NodeModule).getAModuleExportsValue()
-    or
-    exists(ASTNode export | result.getEnclosingExpr() = export | mod.exports(_, export))
-  }
-
   /**
    * A parameter of an exported function, seen as a source for shell command constructed from library input.
    */
   class ExternalInputSource extends Source, DataFlow::ParameterNode {
     ExternalInputSource() {
-      this = getAnExportedValue().(DataFlow::FunctionNode).getAParameter() and
+      this =
+        Exports::getAValueExportedBy(Exports::getTopmostPackageJSON())
+            .(DataFlow::FunctionNode)
+            .getAParameter() and
       not this.getName() = ["cmd", "command"] // looks to be on purpose.
     }
   }

javascript/ql/test/library-tests/PackageExports/index.js

@@ -0,0 +1 @@
+module.exports = function notExporterAnyWhere() {} 

javascript/ql/test/library-tests/PackageExports/lib1/bar.js

@@ -0,0 +1 @@
+module.exports = function notImportedAnywhere() {}

javascript/ql/test/library-tests/PackageExports/lib1/foo.js

@@ -0,0 +1,3 @@
+module.exports = function thisIsRequiredFromMain() {}
+
+module.exports.foo = function alsoExported() {}

javascript/ql/test/library-tests/PackageExports/lib1/index.js

@@ -0,0 +1 @@
+module.exports = function alsoNotExported() {}

javascript/ql/test/library-tests/PackageExports/lib1/main.js

@@ -0,0 +1,17 @@
+module.exports = function isExported() {}
+
+module.exports.foo = require("./foo.js")
+
+module.exports.bar = class Bar {
+    constructor() {} // all are exported
+    static staticMethod() {}
+    instanceMethod() {}
+}
+
+class Baz {
+    constructor() {} // not exported
+    static staticMethod() {} // not exported
+    instanceMethod() {} // exported
+}
+
+module.exports.Baz = new Baz()

javascript/ql/test/library-tests/PackageExports/lib1/package.json

@@ -0,0 +1,3 @@
+{
+    "main": "main.js"
+}

javascript/ql/test/library-tests/PackageExports/lib1/sublib/package.json

@@ -0,0 +1,3 @@
+{
+    "main": "sublib.js"
+}

javascript/ql/test/library-tests/PackageExports/lib1/sublib/sublib.js

@@ -0,0 +1 @@
+module.exports = function exportedInSublibButIsNotAMainPackageExport() {}