Merge pull request github#12189 from erik-krogh/more-export

JS: also consider relative exports when finding library inputs

Author: erik-krogh

javascript/ql/lib/semmle/javascript/NPM.qll

@@ -198,20 +198,20 @@ class PackageJson extends JsonObject {
   /**
    * Gets the main module of this package.
    */
-  Module getMainModule() {
-    result = min(Module m, int prio | m.getFile() = resolveMainModule(this, prio) | m order by prio)
-  }
+  Module getMainModule() { result = this.getExportedModule(".") }
 
   /**
    * Gets the module exported under the given relative path.
    *
    * The main module is considered exported under the path `"."`.
    */
   Module getExportedModule(string relativePath) {
-    relativePath = "." and
-    result = this.getMainModule()
-    or
-    result.getFile() = MainModulePath::of(this, relativePath).resolve()
+    result =
+      min(Module m, int prio |
+        m.getFile() = resolveMainModule(this, prio, relativePath)
+      |
+        m order by prio
+      )
   }
 
   /**

javascript/ql/lib/semmle/javascript/NodeModuleResolutionImpl.qll

@@ -62,7 +62,7 @@ File loadAsFile(Require req, int rootPriority, int priority) {
  */
 File loadAsDirectory(Require req, int rootPriority, int priority) {
   exists(Folder dir | dir = req.getImportedPath().resolve(rootPriority) |
-    result = resolveMainModule(dir.(NpmPackage).getPackageJson(), priority) or
+    result = resolveMainModule(dir.(NpmPackage).getPackageJson(), priority, ".") or
     result = tryExtensions(dir, "index", priority - (numberOfExtensions() + 1))
   )
 }
@@ -132,12 +132,10 @@ private File resolveMainPath(PackageJson pkg, string mainPath, int priority) {
 /**
  * Gets the main module described by `pkg` with the given `priority`.
  */
-File resolveMainModule(PackageJson pkg, int priority) {
-  exists(int subPriority, string mainPath |
-    result = resolveMainPath(pkg, mainPath, subPriority) and
-    if mainPath = "." then subPriority = priority else priority = subPriority + 1000
-  )
+File resolveMainModule(PackageJson pkg, int priority, string exportPath) {
+  result = resolveMainPath(pkg, exportPath, priority)
   or
+  exportPath = "." and
   exists(Folder folder, Folder child |
     child = folder or
     child = folder.getChildContainer(getASrcFolderName()) or
@@ -149,6 +147,7 @@ File resolveMainModule(PackageJson pkg, int priority) {
   )
   or
   // if there is no main module, then we look for files that are explicitly included in the published package.
+  exportPath = "." and
   exists(PathExpr file |
     // `FilesPath` only exists if there is no main module for a given package.
     file = FilesPath::of(pkg) and priority = 100 // fixing the priority, because there might be multiple files in the package.

javascript/ql/lib/semmle/javascript/PackageExports.qll

@@ -133,7 +133,9 @@ private DataFlow::Node getAValueExportedByPackage() {
     DataFlow::globalVarRef("define").getACall().getAnArgument() = factory.getALocalUse() and
     func.getFile() =
       min(int j, File f |
-        f = NodeModule::resolveMainModule(any(PackageJson pack | exists(pack.getPackageName())), j)
+        f =
+          NodeModule::resolveMainModule(any(PackageJson pack | exists(pack.getPackageName())), j,
+            ".")
       |
         f order by j
       )

javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected

@@ -12,6 +12,10 @@ nodes
 | lib2/index.ts:1:28:1:28 | s |
 | lib2/index.ts:2:29:2:29 | s |
 | lib2/index.ts:2:29:2:29 | s |
+| lib2/src/MyNode.ts:1:28:1:28 | s |
+| lib2/src/MyNode.ts:1:28:1:28 | s |
+| lib2/src/MyNode.ts:2:29:2:29 | s |
+| lib2/src/MyNode.ts:2:29:2:29 | s |
 | lib/src/MyNode.ts:1:28:1:28 | s |
 | lib/src/MyNode.ts:1:28:1:28 | s |
 | lib/src/MyNode.ts:2:29:2:29 | s |
@@ -108,6 +112,10 @@ edges
 | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
 | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
 | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
+| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s |
+| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s |
+| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s |
+| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s |
 | lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
 | lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
 | lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
@@ -200,6 +208,7 @@ edges
 | jquery-plugin.js:12:31:12:41 | options.foo | jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:41 | options.foo | This HTML construction which depends on $@ might later allow $@. | jquery-plugin.js:11:34:11:40 | options | library input | jquery-plugin.js:12:20:12:53 | "<span> ... /span>" | cross-site scripting |
 | jquery-plugin.js:14:31:14:35 | stuff | jquery-plugin.js:11:27:11:31 | stuff | jquery-plugin.js:14:31:14:35 | stuff | This HTML construction which depends on $@ might later allow $@. | jquery-plugin.js:11:27:11:31 | stuff | library input | jquery-plugin.js:14:20:14:47 | "<span> ... /span>" | cross-site scripting |
 | lib2/index.ts:2:29:2:29 | s | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | lib2/index.ts:1:28:1:28 | s | library input | lib2/index.ts:3:49:3:52 | html | cross-site scripting |
+| lib2/src/MyNode.ts:2:29:2:29 | s | lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | lib2/src/MyNode.ts:1:28:1:28 | s | library input | lib2/src/MyNode.ts:3:49:3:52 | html | cross-site scripting |
 | lib/src/MyNode.ts:2:29:2:29 | s | lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | lib/src/MyNode.ts:1:28:1:28 | s | library input | lib/src/MyNode.ts:3:49:3:52 | html | cross-site scripting |
 | main.js:2:29:2:29 | s | main.js:1:55:1:55 | s | main.js:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | main.js:1:55:1:55 | s | library input | main.js:3:49:3:52 | html | cross-site scripting |
 | main.js:7:49:7:49 | s | main.js:6:49:6:49 | s | main.js:7:49:7:49 | s | This XML parsing which depends on $@ might later allow $@. | main.js:6:49:6:49 | s | library input | main.js:8:48:8:66 | doc.documentElement | cross-site scripting |

javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/lib2/package.json

@@ -1,6 +1,6 @@
 {
     "name": "my-unsafe-library",
-    "main": "./foobar.js",
+    "main": "./index.ts",
     "exports": {
         "./MyNode": {
             "require": "./lib/MyNode.cjs",

javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/lib2/src/MyNode.ts

@@ -1,4 +1,4 @@
 export function trivialXss(s: string) {
-    const html = "<span>" + s + "</span>"; // OK - this file is not recognized as a main file.
+    const html = "<span>" + s + "</span>"; // NOT OK - this file is not recognized as a main file.
     document.querySelector("#html").innerHTML = html;
 }