Skip to content

Commit 080271f

Browse files
authored
Merge pull request #8221 from erik-krogh/libProto
JS: recognize more module exports from the factory pattern
2 parents fa377ac + 3b0066e commit 080271f

File tree

5 files changed

+36
-5
lines changed

5 files changed

+36
-5
lines changed

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,17 +126,23 @@ private DataFlow::Node getAValueExportedByPackage() {
126126
// }));
127127
// ```
128128
// Such files are not recognized as modules, so we manually use `NodeModule::resolveMainModule` to resolve the file against a `package.json` file.
129-
exists(ImmediatelyInvokedFunctionExpr func, DataFlow::ParameterNode prev, int i |
130-
prev.getName() = "factory" and
131-
func.getParameter(i) = prev.getParameter() and
132-
result = func.getInvocation().getArgument(i).flow().getAFunctionValue().getAReturn() and
133-
DataFlow::globalVarRef("define").getACall().getArgument(1) = prev.getALocalUse() and
129+
exists(ImmediatelyInvokedFunctionExpr func, DataFlow::ParameterNode factory, int i |
130+
factory.getName() = "factory" and
131+
func.getParameter(i) = factory.getParameter() and
132+
DataFlow::globalVarRef("define").getACall().getAnArgument() = factory.getALocalUse() and
134133
func.getFile() =
135134
min(int j, File f |
136135
f = NodeModule::resolveMainModule(any(PackageJson pack | exists(pack.getPackageName())), j)
137136
|
138137
f order by j
139138
)
139+
|
140+
result = func.getInvocation().getArgument(i).flow().getAFunctionValue().getAReturn()
141+
or
142+
exists(DataFlow::ParameterNode exports | exports.getName() = "exports" |
143+
exports = func.getInvocation().getAnArgument().flow().getAFunctionValue().getParameter(0) and
144+
result = exports.getAPropertyWrite().getRhs()
145+
)
140146
)
141147
or
142148
// the exported value is a call to a unique callee

javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
| lib/snapdragon.js:7:28:7:29 | a* | Strings starting with 'a' and with many repetitions of 'a' can start matching anywhere after the start of the preceeding aa*$ |
3838
| lib/snapdragon.js:15:26:15:27 | a* | Strings starting with 'a' and with many repetitions of 'a' can start matching anywhere after the start of the preceeding aa*$ |
3939
| lib/snapdragon.js:23:22:23:23 | a* | Strings starting with 'a' and with many repetitions of 'a' can start matching anywhere after the start of the preceeding aa*$ |
40+
| lib/subLib4/factory.js:8:3:8:4 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
4041
| 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 |
4142
| polynomial-redos.js:7:24:7:26 | \\s+ | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s+$ |
4243
| polynomial-redos.js:8:17:8:18 | * | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding *, * |

javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ nodes
4747
| lib/snapdragon.js:23:5:23:12 | node.val |
4848
| lib/snapdragon.js:23:5:23:12 | node.val |
4949
| lib/snapdragon.js:25:22:25:26 | input |
50+
| lib/subLib4/factory.js:7:27:7:30 | name |
51+
| lib/subLib4/factory.js:7:27:7:30 | name |
52+
| lib/subLib4/factory.js:8:13:8:16 | name |
53+
| lib/subLib4/factory.js:8:13:8:16 | name |
5054
| lib/sublib/factory.js:12:26:12:29 | name |
5155
| lib/sublib/factory.js:12:26:12:29 | name |
5256
| lib/sublib/factory.js:13:24:13:27 | name |
@@ -238,6 +242,10 @@ edges
238242
| lib/snapdragon.js:23:5:23:8 | node | lib/snapdragon.js:23:5:23:12 | node.val |
239243
| lib/snapdragon.js:23:5:23:8 | node | lib/snapdragon.js:23:5:23:12 | node.val |
240244
| lib/snapdragon.js:25:22:25:26 | input | lib/snapdragon.js:22:44:22:47 | node |
245+
| lib/subLib4/factory.js:7:27:7:30 | name | lib/subLib4/factory.js:8:13:8:16 | name |
246+
| lib/subLib4/factory.js:7:27:7:30 | name | lib/subLib4/factory.js:8:13:8:16 | name |
247+
| lib/subLib4/factory.js:7:27:7:30 | name | lib/subLib4/factory.js:8:13:8:16 | name |
248+
| lib/subLib4/factory.js:7:27:7:30 | name | lib/subLib4/factory.js:8:13:8:16 | name |
241249
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
242250
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
243251
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
@@ -389,6 +397,7 @@ edges
389397
| lib/snapdragon.js:7:15:7:32 | this.match(/aa*$/) | lib/snapdragon.js:3:34:3:38 | input | lib/snapdragon.js:7:15:7:18 | this | This $@ that depends on $@ may run slow on strings starting with 'a' and with many repetitions of 'a'. | lib/snapdragon.js:7:28:7:29 | a* | regular expression | lib/snapdragon.js:3:34:3:38 | input | library input |
390398
| lib/snapdragon.js:15:13:15:30 | this.match(/aa*$/) | lib/snapdragon.js:12:34:12:38 | input | lib/snapdragon.js:15:13:15:16 | this | This $@ that depends on $@ may run slow on strings starting with 'a' and with many repetitions of 'a'. | lib/snapdragon.js:15:26:15:27 | a* | regular expression | lib/snapdragon.js:12:34:12:38 | input | library input |
391399
| lib/snapdragon.js:23:5:23:26 | node.va ... /aa*$/) | lib/snapdragon.js:20:34:20:38 | input | lib/snapdragon.js:23:5:23:12 | node.val | This $@ that depends on $@ may run slow on strings starting with 'a' and with many repetitions of 'a'. | lib/snapdragon.js:23:22:23:23 | a* | regular expression | lib/snapdragon.js:20:34:20:38 | input | library input |
400+
| lib/subLib4/factory.js:8:2:8:17 | /f*g/.test(name) | lib/subLib4/factory.js:7:27:7:30 | name | lib/subLib4/factory.js:8:13:8:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/subLib4/factory.js:8:3:8:4 | f* | regular expression | lib/subLib4/factory.js:7:27:7:30 | name | library input |
392401
| 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 |
393402
| 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 |
394403
| 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 |
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
(function (global, factory) {
2+
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
3+
typeof define === 'function' && define.amd ? define('some-lib', ['exports'], factory) :
4+
(global = global || self, factory(global.JSData = {}));
5+
}(this, (function (exports) { 'use strict';
6+
7+
exports.foo = function (name) {
8+
/f*g/.test(name); // NOT OK
9+
}
10+
})));
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "my-sub-lib",
3+
"version": "0.0.7",
4+
"main": "./factory.js"
5+
}

0 commit comments

Comments
 (0)