Merge pull request github#5439 from erik-krogh/topPack

Approved by esbena

Author: codeql-ci

javascript/ql/src/Security/CWE-078/UnsafeShellCommandConstruction.ql

@@ -18,6 +18,6 @@ import DataFlow::PathGraph
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
 where cfg.hasFlowPath(source, sink) and sinkNode = sink.getNode()
-select sinkNode.getAlertLocation(), source, sink, "$@ based on library input is later used in $@.",
-  sinkNode.getAlertLocation(), sinkNode.getSinkType(), sinkNode.getCommandExecution(),
-  "shell command"
+select sinkNode.getAlertLocation(), source, sink, "$@ based on $@ is later used in $@.",
+  sinkNode.getAlertLocation(), sinkNode.getSinkType(), source.getNode(), "library input",
+  sinkNode.getCommandExecution(), "shell command"

javascript/ql/src/semmle/javascript/NodeModuleResolutionImpl.qll

@@ -96,8 +96,11 @@ File resolveMainModule(PackageJSON pkg, int priority) {
     )
   )
   or
-  result =
-    tryExtensions(pkg.getFile().getParentContainer(), "index", priority - prioritiesPerCandidate())
+  exists(Folder folder | folder = pkg.getFile().getParentContainer() |
+    result =
+      tryExtensions([folder, folder.getChildContainer(["src", "lib"])], "index",
+        priority - prioritiesPerCandidate())
+  )
 }
 
 /**

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

@@ -17,33 +17,12 @@ DataFlow::ParameterNode getALibraryInputParameter() {
 }
 
 /**
- * Gets the number of occurrences of "/" in `path`.
- */
-bindingset[path]
-private int countSlashes(string path) { result = count(path.splitAt("/")) - 1 }
-
-/**
- * Gets the topmost named 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 and
-      exists(j.getPackageName())
-    |
-      j order by countSlashes(j.getFile().getRelativePath())
-    )
-}
-
-/**
- * Gets a value exported by the main module from one of the topmost `package.json` files (see `getTopmostPackageJSON`).
+ * Gets a value exported by the main module from a named `package.json` file.
  * 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 getAValueExportedByPackage() {
-  result = getAnExportFromModule(getTopmostPackageJSON().getMainModule())
+  result =
+    getAnExportFromModule(any(PackageJSON pack | exists(pack.getPackageName())).getMainModule())
   or
   result = getAValueExportedByPackage().(DataFlow::PropWrite).getRhs()
   or

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

@@ -646,8 +646,6 @@ module NodeJSLib {
     }
   }
 
-  private import semmle.javascript.PackageExports as Exports
-
   /**
    * A direct step from an named export to a property-read reading the exported value.
    */

javascript/ql/test/library-tests/PackageExports/tests.expected

@@ -1,6 +1,3 @@
-getTopmostPackageJSON
-| absent_main/package.json:1:1:4:1 | {\\n    " ... t.js"\\n} |
-| lib1/package.json:1:1:4:1 | {\\n    " ... n.js"\\n} |
 getALibraryInputParameter
 | lib1/foo.js:3:44:3:44 | d |
 | lib1/main.js:6:17:6:17 | a |

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

@@ -1,8 +1,6 @@
 import javascript
 import semmle.javascript.PackageExports as Exports
 
-query PackageJSON getTopmostPackageJSON() { result = Exports::getTopmostPackageJSON() }
-
 query DataFlow::Node getALibraryInputParameter() { result = Exports::getALibraryInputParameter() }
 
 query DataFlow::Node getAnExportedValue(Module mod, string name) {

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected

@@ -1,7 +1,7 @@
 var cp = require("child_process")
 
 module.exports = function (name) {
-	cp.exec("rm -rf " + name); // OK - this file belongs in a sub-"module", and is not the primary exported module.
+	cp.exec("rm -rf " + name); // NOT OK - functions exported as part of a submodule are also flagged.
 };
 
 module.exports.foo = function (name) {