From fbfbe70debe926bf1ed14f595f7b2a2da91f75d0 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 19 Jan 2021 20:31:28 +0100 Subject: [PATCH 1/2] add support for unnamed/default exports in PackageExports.qll --- javascript/ql/src/semmle/javascript/PackageExports.qll | 7 +++++++ .../Performance/ReDoS/PolynomialBackTracking.expected | 1 + .../Performance/ReDoS/PolynomialReDoS.expected | 9 +++++++++ .../ql/test/query-tests/Performance/ReDoS/lib/lib.js | 8 ++++++++ 4 files changed, 25 insertions(+) diff --git a/javascript/ql/src/semmle/javascript/PackageExports.qll b/javascript/ql/src/semmle/javascript/PackageExports.qll index 09212bc8aa6..350a6301dd3 100644 --- a/javascript/ql/src/semmle/javascript/PackageExports.qll +++ b/javascript/ql/src/semmle/javascript/PackageExports.qll @@ -78,5 +78,12 @@ private DataFlow::Node getAValueExportedByPackage() { private DataFlow::Node getAnExportFromModule(Module mod) { result.analyze().getAValue() = mod.(NodeModule).getAModuleExportsValue() or + exists(Variable var | var = mod.(Closure::ClosureModule).getExportsVariable() | + result.asExpr() = var.getAReference() or + result.asExpr() = var.getAnAssignedExpr() + ) + or + result.analyze().getAValue() = mod.(AmdModule).getDefine().getAModuleExportsValue() + or result = mod.getAnExportedValue(_) } diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected index 1c465b163dd..fcd8c43cd7c 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected +++ b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected @@ -25,6 +25,7 @@ | highlight.js:38:64:38:69 | [^()]* | Strings starting with 'A(' and with many repetitions of ''' can start matching anywhere after the start of the preceeding [^()]* | | highlight.js:39:22:39:24 | \\w* | Strings starting with 'A' and with many repetitions of 'A' can start matching anywhere after the start of the preceeding [a-zA-Z_]\\w*\\([^()]*(\\([^()]*(\\([^()]*\\)[^()]*)*\\)[^()]*)*\\)\\s*\\{ | | 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 | | 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* | diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected index 59428ed86e0..9e5619f1aa3 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected +++ b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected @@ -3,6 +3,10 @@ nodes | lib/lib.js:3:28:3:31 | name | | lib/lib.js:4:14:4:17 | name | | lib/lib.js:4:14:4:17 | name | +| lib/lib.js:7:19:7:22 | name | +| lib/lib.js:7:19:7:22 | name | +| lib/lib.js:8:13:8:16 | name | +| lib/lib.js:8:13:8:16 | 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 | @@ -150,6 +154,10 @@ edges | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | 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/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 | | 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 | @@ -289,6 +297,7 @@ edges | polynomial-redos.js:123:13:123:20 | replaced | polynomial-redos.js:123:3:123:20 | result | #select | 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 | | 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 | diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js b/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js index 23ba27a32c2..47fc928845b 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js +++ b/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js @@ -3,3 +3,11 @@ var regexp = /a*b/; module.exports = function (name) { regexp.test(name); // NOT OK }; + +function bar(reg, name) { + /f*g/.test(name); // NOT OK +} + +if (typeof define !== 'undefined' && define.amd) { // AMD + define([], function () {return bar}); +} \ No newline at end of file From a44aefa6c987609748edd6d00beb260ef7a8ef23 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Wed, 20 Jan 2021 14:54:39 +0100 Subject: [PATCH 2/2] add test for top-level closure modules - and simplify --- javascript/ql/src/semmle/javascript/PackageExports.qll | 5 +---- .../Performance/ReDoS/PolynomialBackTracking.expected | 1 + .../Performance/ReDoS/PolynomialReDoS.expected | 9 +++++++++ .../ql/test/query-tests/Performance/ReDoS/lib/closure.js | 5 +++++ .../ql/test/query-tests/Performance/ReDoS/lib/lib.js | 4 +++- 5 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 javascript/ql/test/query-tests/Performance/ReDoS/lib/closure.js diff --git a/javascript/ql/src/semmle/javascript/PackageExports.qll b/javascript/ql/src/semmle/javascript/PackageExports.qll index 350a6301dd3..bdb0c297411 100644 --- a/javascript/ql/src/semmle/javascript/PackageExports.qll +++ b/javascript/ql/src/semmle/javascript/PackageExports.qll @@ -78,10 +78,7 @@ private DataFlow::Node getAValueExportedByPackage() { private DataFlow::Node getAnExportFromModule(Module mod) { result.analyze().getAValue() = mod.(NodeModule).getAModuleExportsValue() or - exists(Variable var | var = mod.(Closure::ClosureModule).getExportsVariable() | - result.asExpr() = var.getAReference() or - result.asExpr() = var.getAnAssignedExpr() - ) + result = mod.(Closure::ClosureModule).getExportsVariable().getAnAssignedExpr().flow() or result.analyze().getAValue() = mod.(AmdModule).getDefine().getAModuleExportsValue() or diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected index fcd8c43cd7c..8452d81ee07 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected +++ b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected @@ -24,6 +24,7 @@ | highlight.js:38:54:38:59 | [^()]* | Strings starting with 'A((' and with many repetitions of ''' can start matching anywhere after the start of the preceeding [^()]* | | highlight.js:38:64:38:69 | [^()]* | Strings starting with 'A(' and with many repetitions of ''' can start matching anywhere after the start of the preceeding [^()]* | | highlight.js:39:22:39:24 | \\w* | Strings starting with 'A' and with many repetitions of 'A' can start matching anywhere after the start of the preceeding [a-zA-Z_]\\w*\\([^()]*(\\([^()]*(\\([^()]*\\)[^()]*)*\\)[^()]*)*\\)\\s*\\{ | +| 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 | | polynomial-redos.js:7:24:7:26 | \\s+ | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s+$ | diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected index 9e5619f1aa3..d40e70d0fc6 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected +++ b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected @@ -1,4 +1,8 @@ nodes +| lib/closure.js:3:21:3:21 | x | +| lib/closure.js:3:21:3:21 | x | +| lib/closure.js:4:16:4:16 | x | +| lib/closure.js:4:16:4:16 | x | | lib/lib.js:3:28:3:31 | name | | lib/lib.js:3:28:3:31 | name | | lib/lib.js:4:14:4:17 | name | @@ -150,6 +154,10 @@ nodes | polynomial-redos.js:124:12:124:17 | result | | polynomial-redos.js:124:12:124:17 | result | edges +| lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x | +| lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x | +| lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x | +| lib/closure.js:3:21:3:21 | x | lib/closure.js:4:16:4:16 | x | | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | @@ -296,6 +304,7 @@ edges | polynomial-redos.js:123:3:123:20 | result | polynomial-redos.js:124:12:124:17 | result | | polynomial-redos.js:123:13:123:20 | replaced | polynomial-redos.js:123:3:123:20 | result | #select +| 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 | | 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 | diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/lib/closure.js b/javascript/ql/test/query-tests/Performance/ReDoS/lib/closure.js new file mode 100644 index 00000000000..4f5ca816870 --- /dev/null +++ b/javascript/ql/test/query-tests/Performance/ReDoS/lib/closure.js @@ -0,0 +1,5 @@ +goog.module('x.y.z.closure2'); + +exports = function (x) { + /u*o/.test(x); // NOT OK +} \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js b/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js index 47fc928845b..d18f43078c3 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js +++ b/javascript/ql/test/query-tests/Performance/ReDoS/lib/lib.js @@ -10,4 +10,6 @@ function bar(reg, name) { if (typeof define !== 'undefined' && define.amd) { // AMD define([], function () {return bar}); -} \ No newline at end of file +} + +module.exports.closure = require("./closure") \ No newline at end of file