Skip to content

Commit 69cd9df

Browse files
authored
Merge pull request github#5826 from erik-krogh/moreLib
Approved by esbena
2 parents 390ee3a + e333267 commit 69cd9df

File tree

5 files changed

+87
-1
lines changed

5 files changed

+87
-1
lines changed

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

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,23 @@ DataFlow::ParameterNode getALibraryInputParameter() {
1919
)
2020
}
2121

22+
private import NodeModuleResolutionImpl as NodeModule
23+
2224
/**
2325
* Gets a value exported by the main module from a named `package.json` file.
24-
* The value is either directly the `module.exports` value, a nested property of `module.exports`, or a method on an exported class.
2526
*/
2627
private DataFlow::Node getAValueExportedByPackage() {
28+
// The base case, an export from a named `package.json` file.
2729
result =
2830
getAnExportFromModule(any(PackageJSON pack | exists(pack.getPackageName())).getMainModule())
2931
or
32+
// module.exports.bar.baz = result;
3033
result = getAValueExportedByPackage().(DataFlow::PropWrite).getRhs()
3134
or
35+
// class Foo {
36+
// bar() {} // <- result
37+
// };
38+
// module.exports = new Foo();
3239
exists(DataFlow::SourceNode callee |
3340
callee = getAValueExportedByPackage().(DataFlow::NewNode).getCalleeNode().getALocalSource()
3441
|
@@ -39,20 +46,67 @@ private DataFlow::Node getAValueExportedByPackage() {
3946
or
4047
result = getAValueExportedByPackage().getALocalSource()
4148
or
49+
// Nested property reads.
4250
result = getAValueExportedByPackage().(DataFlow::SourceNode).getAPropertyReference()
4351
or
52+
// module.exports.foo = require("./other-module.js");
4453
exists(Module mod |
4554
mod = getAValueExportedByPackage().getEnclosingExpr().(Import).getImportedModule()
4655
|
4756
result = getAnExportFromModule(mod)
4857
)
4958
or
59+
// module.exports = class Foo {
60+
// bar() {} // <- result
61+
// static baz() {} // <- result
62+
// constructor() {} // <- result
63+
// };
5064
exists(DataFlow::ClassNode cla | cla = getAValueExportedByPackage() |
5165
result = cla.getAnInstanceMethod() or
5266
result = cla.getAStaticMethod() or
5367
result = cla.getConstructor()
5468
)
5569
or
70+
// One shot closures that define a "factory" function.
71+
// Recognizes the following pattern:
72+
// ```Javascript
73+
// (function (root, factory) {
74+
// if (typeof define === 'function' && define.amd) {
75+
// define('library-name', factory);
76+
// } else if (typeof exports === 'object') {
77+
// module.exports = factory();
78+
// } else {
79+
// root.libraryName = factory();
80+
// }
81+
// }(this, function () {
82+
// ....
83+
// }));
84+
// ```
85+
// Such files are not recognized as modules, so we manually use `NodeModule::resolveMainModule` to resolve the file against a `package.json` file.
86+
exists(ImmediatelyInvokedFunctionExpr func, DataFlow::ParameterNode prev, int i |
87+
prev.getName() = "factory" and
88+
func.getParameter(i) = prev.getParameter() and
89+
result = func.getInvocation().getArgument(i).flow().getAFunctionValue().getAReturn() and
90+
DataFlow::globalVarRef("define").getACall().getArgument(1) = prev.getALocalUse() and
91+
func.getFile() =
92+
min(int j, File f |
93+
f = NodeModule::resolveMainModule(any(PackageJSON pack | exists(pack.getPackageName())), j)
94+
|
95+
f order by j
96+
)
97+
)
98+
or
99+
// the exported value is a call to a unique callee
100+
// ```JavaScript
101+
// module.exports = foo();
102+
// function foo() {
103+
// return result;
104+
// }
105+
// ```
106+
exists(DataFlow::CallNode call | call = getAValueExportedByPackage() |
107+
result = unique( | | call.getCalleeNode().getAFunctionValue()).getAReturn()
108+
)
109+
or
56110
// *****
57111
// Common styles of transforming exported objects.
58112
// *****

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
| 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 |
3131
| 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 |
3232
| 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 |
33+
| 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 |
3334
| polynomial-redos.js:7:24:7:26 | \\s+ | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s+$ |
3435
| polynomial-redos.js:8:17:8:18 | * | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding *, * |
3536
| 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* |

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ nodes
1111
| lib/lib.js:7:19:7:22 | name |
1212
| lib/lib.js:8:13:8:16 | name |
1313
| lib/lib.js:8:13:8:16 | name |
14+
| lib/sublib/factory.js:12:26:12:29 | name |
15+
| lib/sublib/factory.js:12:26:12:29 | name |
16+
| lib/sublib/factory.js:13:24:13:27 | name |
17+
| lib/sublib/factory.js:13:24:13:27 | name |
1418
| polynomial-redos.js:5:6:5:32 | tainted |
1519
| polynomial-redos.js:5:16:5:32 | req.query.tainted |
1620
| polynomial-redos.js:5:16:5:32 | req.query.tainted |
@@ -166,6 +170,10 @@ edges
166170
| lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
167171
| lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
168172
| lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name |
173+
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
174+
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
175+
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
176+
| lib/sublib/factory.js:12:26:12:29 | name | lib/sublib/factory.js:13:24:13:27 | name |
169177
| polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
170178
| polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
171179
| polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:8:2:8:8 | tainted |
@@ -307,6 +315,7 @@ edges
307315
| 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 |
308316
| 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 |
309317
| 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 |
318+
| 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 |
310319
| 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 |
311320
| 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 |
312321
| 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 |
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
2+
(function (root, factory) {
3+
if (typeof define === 'function' && define.amd) {
4+
define('my-sub-library', factory);
5+
} else if (typeof exports === 'object') {
6+
module.exports = factory();
7+
} else {
8+
root.mySubLibrary = factory();
9+
}
10+
}(this, function () {
11+
function create() {
12+
return function (name) {
13+
/f*g/.test(name); // NOT OK
14+
}
15+
}
16+
return create()
17+
}));
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)