Merge pull request #5826 from erik-krogh/moreLib

Approved by esbena
2026-04-30 11:15:13 +02:00 · 2021-05-05 04:40:49 -07:00
parent 390ee3a6b8 e333267e69
commit 69cd9dfb7d
5 changed files with 87 additions and 1 deletions
--- a/javascript/ql/src/semmle/javascript/PackageExports.qll
+++ b/javascript/ql/src/semmle/javascript/PackageExports.qll
@@ -19,16 +19,23 @@ DataFlow::ParameterNode getALibraryInputParameter() {
  )
 }

+private import NodeModuleResolutionImpl as NodeModule
+
 /**
 * 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() {
+  // The base case, an export from a named `package.json` file.
  result =
    getAnExportFromModule(any(PackageJSON pack | exists(pack.getPackageName())).getMainModule())
  or
+  // module.exports.bar.baz = result;
  result = getAValueExportedByPackage().(DataFlow::PropWrite).getRhs()
  or
+  // class Foo {
+  //   bar() {} // <- result
+  // };
+  // module.exports = new Foo();
  exists(DataFlow::SourceNode callee |
    callee = getAValueExportedByPackage().(DataFlow::NewNode).getCalleeNode().getALocalSource()
  |
@@ -39,20 +46,67 @@ private DataFlow::Node getAValueExportedByPackage() {
  or
  result = getAValueExportedByPackage().getALocalSource()
  or
+  // Nested property reads.
  result = getAValueExportedByPackage().(DataFlow::SourceNode).getAPropertyReference()
  or
+  // module.exports.foo = require("./other-module.js");
  exists(Module mod |
    mod = getAValueExportedByPackage().getEnclosingExpr().(Import).getImportedModule()
  |
    result = getAnExportFromModule(mod)
  )
  or
+  // module.exports = class Foo {
+  //   bar() {} // <- result
+  //   static baz() {} // <- result
+  //   constructor() {} // <- result
+  // };
  exists(DataFlow::ClassNode cla | cla = getAValueExportedByPackage() |
    result = cla.getAnInstanceMethod() or
    result = cla.getAStaticMethod() or
    result = cla.getConstructor()
  )
  or
+  // One shot closures that define a "factory" function.
+  // Recognizes the following pattern:
+  // ```Javascript
+  // (function (root, factory) {
+  //   if (typeof define === 'function' && define.amd) {
+  //     define('library-name', factory);
+  //   } else if (typeof exports === 'object') {
+  //     module.exports = factory();
+  //   } else {
+  //     root.libraryName = factory();
+  //   }
+  // }(this, function () {
+  //   ....
+  // }));
+  // ```
+  // Such files are not recognized as modules, so we manually use `NodeModule::resolveMainModule` to resolve the file against a `package.json` file.
+  exists(ImmediatelyInvokedFunctionExpr func, DataFlow::ParameterNode prev, int i |
+    prev.getName() = "factory" and
+    func.getParameter(i) = prev.getParameter() and
+    result = func.getInvocation().getArgument(i).flow().getAFunctionValue().getAReturn() and
+    DataFlow::globalVarRef("define").getACall().getArgument(1) = prev.getALocalUse() and
+    func.getFile() =
+      min(int j, File f |
+        f = NodeModule::resolveMainModule(any(PackageJSON pack | exists(pack.getPackageName())), j)
+      |
+        f order by j
+      )
+  )
+  or
+  // the exported value is a call to a unique callee
+  // ```JavaScript
+  // module.exports = foo();
+  // function foo() {
+  //   return result;
+  // }
+  // ```
+  exists(DataFlow::CallNode call | call = getAValueExportedByPackage() |
+    result = unique( | | call.getCalleeNode().getAFunctionValue()).getAReturn()
+  )
+  or
  // *****
  // Common styles of transforming exported objects.
  // *****
--- a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected
+++ b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected
@@ -30,6 +30,7 @@
 | lib/closure.js:4:6:4:7 | u* | Strings with many repetitions of 'u' can start matching anywhere after the start of the preceeding u*o |
 | lib/lib.js:1:15:1:16 | a* | Strings with many repetitions of 'a' can start matching anywhere after the start of the preceeding a*b |
 | lib/lib.js:8:3:8:4 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
+| lib/sublib/factory.js:13:14:13:15 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
 | polynomial-redos.js:7:24:7:26 | \\s+ | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s+$ |
 | polynomial-redos.js:8:17:8:18 |  * | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding  *, * |
 | polynomial-redos.js:9:19:9:21 | \\s* | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s*\\n\\s* |
--- a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected
+++ b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected
@@ -11,6 +11,10 @@ nodes
 | lib/lib.js:7:19:7:22 | name |
 | lib/lib.js:8:13:8:16 | name |
 | lib/lib.js:8:13:8:16 | name |
+| lib/sublib/factory.js:12:26:12:29 | name |
+| lib/sublib/factory.js:12:26:12:29 | name |
+| lib/sublib/factory.js:13:24:13:27 | name |
+| lib/sublib/factory.js:13:24:13:27 | name |
 | polynomial-redos.js:5:6:5:32 | tainted |
 | polynomial-redos.js:5:16:5:32 | req.query.tainted |
 | polynomial-redos.js:5:16:5:32 | req.query.tainted |
@@ -166,6 +170,10 @@ edges
 | lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
 | lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
 | lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
+| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
+| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
+| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
+| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:8:2:8:8 | tainted |
@@ -307,6 +315,7 @@ edges
 | lib/closure.js:4:5:4:17 | /u*o/.test(x) | lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x | This $@ that depends on $@ may run slow on strings with many repetitions of 'u'. | lib/closure.js:4:6:4:7 | u* | regular expression | lib/closure.js:3:21:3:21 | x | library input |
 | lib/lib.js:4:2:4:18 | regexp.test(name) | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/lib.js:1:15:1:16 | a* | regular expression | lib/lib.js:3:28:3:31 | name | library input |
 | lib/lib.js:8:2:8:17 | /f*g/.test(name) | lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:8:3:8:4 | f* | regular expression | lib/lib.js:7:19:7:22 | name | library input |
+| lib/sublib/factory.js:13:13:13:28 | /f*g/.test(name) | lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/sublib/factory.js:13:14:13:15 | f* | regular expression | lib/sublib/factory.js:12:26:12:29 | name | library input |
 | polynomial-redos.js:7:2:7:34 | tainted ... /g, '') | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:7:2:7:8 | tainted | This $@ that depends on $@ may run slow on strings with many repetitions of ' '. | polynomial-redos.js:7:24:7:26 | \\s+ | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
 | polynomial-redos.js:8:2:8:23 | tainted ...  *, */) | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:8:2:8:8 | tainted | This $@ that depends on $@ may run slow on strings with many repetitions of ' '. | polynomial-redos.js:8:17:8:18 |  * | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
 | polynomial-redos.js:9:2:9:34 | tainted ... g, ' ') | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:9:2:9:8 | tainted | This $@ that depends on $@ may run slow on strings with many repetitions of ' '. | polynomial-redos.js:9:19:9:21 | \\s* | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
--- a/javascript/ql/test/query-tests/Performance/ReDoS/lib/sublib/factory.js
+++ b/javascript/ql/test/query-tests/Performance/ReDoS/lib/sublib/factory.js
@@ -0,0 +1,17 @@
+
+(function (root, factory) {
+    if (typeof define === 'function' && define.amd) {
+        define('my-sub-library', factory);
+    } else if (typeof exports === 'object') {
+        module.exports = factory();
+    } else {
+        root.mySubLibrary = factory();
+    }
+}(this, function () {
+    function create() {
+        return function (name) {
+            /f*g/.test(name); // NOT OK
+        }
+    }
+    return create()
+}));
--- a/javascript/ql/test/query-tests/Performance/ReDoS/lib/sublib/package.json
+++ b/javascript/ql/test/query-tests/Performance/ReDoS/lib/sublib/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "my-sub-lib",
+  "version": "0.0.7",
+  "main": "./factory.js"
+}