Skip to content

Commit 7a5aae7

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request github#3630 from erik-krogh/DevServer
Approved by asgerf
2 parents 287bc40 + e46bd70 commit 7a5aae7

File tree

9 files changed

+100
-2
lines changed

9 files changed

+100
-2
lines changed

‎change-notes/1.25/analysis-javascript.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
- [sqlite](https://www.npmjs.com/package/sqlite)
2121
- [ssh2-streams](https://www.npmjs.com/package/ssh2-streams)
2222
- [ssh2](https://www.npmjs.com/package/ssh2)
23+
- [webpack-dev-server](https://www.npmjs.com/package/webpack-dev-server)
2324

2425
* TypeScript 3.9 is now supported.
2526

‎javascript/ql/src/semmle/javascript/Modules.qll

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,8 @@ abstract class Import extends ASTNode {
174174
result = resolveAsProvidedModule() or
175175
result = resolveImportedPath() or
176176
result = resolveFromTypeRoot() or
177-
result = resolveFromTypeScriptSymbol()
177+
result = resolveFromTypeScriptSymbol() or
178+
result = resolveNeighbourPackage(this.getImportedPath().getValue())
178179
)
179180
}
180181

@@ -196,3 +197,28 @@ abstract deprecated class PathExprInModule extends PathExpr {
196197
this.(Comment).getTopLevel() instanceof Module
197198
}
198199
}
200+
201+
/**
202+
* Gets a module imported from another package in the same repository.
203+
*
204+
* No support for importing from folders inside the other package.
205+
*/
206+
private Module resolveNeighbourPackage(PathString importPath) {
207+
exists(PackageJSON json | importPath = json.getPackageName() and result = json.getMainModule())
208+
or
209+
exists(string package |
210+
result.getFile().getParentContainer() = getPackageFolder(package) and
211+
importPath = package + "/" + [result.getFile().getBaseName(), result.getFile().getStem()]
212+
)
213+
}
214+
215+
/**
216+
* Gets the folder for a package that has name `package` according to a package.json file in the resulting folder.
217+
*/
218+
pragma[noinline]
219+
private Folder getPackageFolder(string package) {
220+
exists(PackageJSON json |
221+
json.getPackageName() = package and
222+
result = json.getFile().getParentContainer()
223+
)
224+
}

‎javascript/ql/src/semmle/javascript/frameworks/Express.qll

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ module Express {
4444
isRouter(e, _)
4545
or
4646
e.getType().hasUnderlyingType("express", "Router")
47+
or
48+
// created by `webpack-dev-server`
49+
WebpackDevServer::webpackDevServerApp().flowsToExpr(e)
4750
}
4851

4952
/**
@@ -903,4 +906,32 @@ module Express {
903906

904907
override DataFlow::ValueNode getARouteHandlerArg() { result = routeHandlerArg }
905908
}
909+
910+
private module WebpackDevServer {
911+
/**
912+
* Gets a source for the options given to an instantiation of `webpack-dev-server`.
913+
*/
914+
private DataFlow::SourceNode devServerOptions(DataFlow::TypeBackTracker t) {
915+
t.start() and
916+
result =
917+
DataFlow::moduleImport("webpack-dev-server")
918+
.getAnInstantiation()
919+
.getArgument(1)
920+
.getALocalSource()
921+
or
922+
exists(DataFlow::TypeBackTracker t2 | result = devServerOptions(t2).backtrack(t2, t))
923+
}
924+
925+
/**
926+
* Gets an instance of the `express` app created by `webpack-dev-server`.
927+
*/
928+
DataFlow::ParameterNode webpackDevServerApp() {
929+
result =
930+
devServerOptions(DataFlow::TypeBackTracker::end())
931+
.getAPropertyWrite(["after", "before", "setup"])
932+
.getRhs()
933+
.getAFunctionValue()
934+
.getParameter(0)
935+
}
936+
}
906937
}

‎javascript/ql/test/library-tests/Portals/PortalEntry.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,12 +711,14 @@
711711
| (parameter 0 (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:7:5:10 | "me" | false |
712712
| (parameter 0 (member foo (root https://www.npmjs.com/package/m2))) | src/m3/tst2.js:5:5:5:12 | { x: o } | false |
713713
| (parameter 0 (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
714+
| (parameter 0 (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:15:4:18 | "hi" | true |
714715
| (parameter 0 (member m (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
715716
| (parameter 0 (member m (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:2:5:2:8 | "hi" | false |
716717
| (parameter 0 (member m (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
717718
| (parameter 0 (member m (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
718719
| (parameter 0 (member m (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:2:5:2:8 | "hi" | false |
719720
| (parameter 0 (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:15:5:21 | "there" | false |
721+
| (parameter 0 (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:15:5:21 | "there" | true |
720722
| (parameter 0 (member s (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:15:5:21 | "there" | false |
721723
| (parameter 0 (member s (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:3:5:3:11 | "there" | false |
722724
| (parameter 0 (member s (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:15:5:21 | "there" | false |

‎javascript/ql/test/library-tests/Portals/PortalExit.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,23 @@
66
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m2/main.js:11:4:11:3 | this | true |
77
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m2/main.js:15:11:15:10 | this | true |
88
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
9+
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:11 | new A("me") | true |
910
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
11+
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | true |
1012
| (instance (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
1113
| (instance (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
1214
| (member default (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:1:8:1:8 | A | false |
1315
| (member foo (root https://www.npmjs.com/package/m2)) | src/m3/tst2.js:1:10:1:12 | foo | false |
1416
| (member m (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
17+
| (member m (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | true |
1518
| (member m (instance (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
1619
| (member m (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:2:1:2:3 | A.m | false |
1720
| (member m (return (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
1821
| (member m (return (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
1922
| (member m (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:2:1:2:3 | A.m | false |
2023
| (member name (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m2/main.js:12:27:12:35 | this.name | true |
2124
| (member s (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | false |
25+
| (member s (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | true |
2226
| (member s (instance (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | false |
2327
| (member s (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:3:1:3:3 | A.s | false |
2428
| (member s (return (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | false |
@@ -734,12 +738,14 @@
734738
| (return (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
735739
| (return (member foo (root https://www.npmjs.com/package/m2))) | src/m3/tst2.js:5:1:5:13 | foo({ x: o }) | false |
736740
| (return (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
741+
| (return (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | true |
737742
| (return (member m (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
738743
| (return (member m (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:2:1:2:9 | A.m("hi") | false |
739744
| (return (member m (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
740745
| (return (member m (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
741746
| (return (member m (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:2:1:2:9 | A.m("hi") | false |
742747
| (return (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
748+
| (return (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | true |
743749
| (return (member s (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
744750
| (return (member s (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:3:1:3:12 | A.s("there") | false |
745751
| (return (member s (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |

‎javascript/ql/test/query-tests/Security/CWE-078/CommandInjection.expected

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ nodes
4848
| child_process-test.js:70:25:70:31 | req.url |
4949
| child_process-test.js:72:29:72:31 | cmd |
5050
| child_process-test.js:72:29:72:31 | cmd |
51+
| child_process-test.js:80:19:80:36 | req.query.fileName |
52+
| child_process-test.js:80:19:80:36 | req.query.fileName |
53+
| child_process-test.js:80:19:80:36 | req.query.fileName |
54+
| child_process-test.js:82:37:82:54 | req.query.fileName |
55+
| child_process-test.js:82:37:82:54 | req.query.fileName |
5156
| execSeries.js:3:20:3:22 | arr |
5257
| execSeries.js:6:14:6:16 | arr |
5358
| execSeries.js:6:14:6:21 | arr[i++] |
@@ -64,6 +69,10 @@ nodes
6469
| execSeries.js:18:34:18:40 | req.url |
6570
| execSeries.js:19:12:19:16 | [cmd] |
6671
| execSeries.js:19:13:19:15 | cmd |
72+
| lib/subLib/index.js:7:32:7:35 | name |
73+
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
74+
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
75+
| lib/subLib/index.js:8:22:8:25 | name |
6776
| other.js:5:9:5:49 | cmd |
6877
| other.js:5:15:5:38 | url.par ... , true) |
6978
| other.js:5:15:5:44 | url.par ... ).query |
@@ -152,6 +161,9 @@ edges
152161
| child_process-test.js:70:15:70:49 | url.par ... ry.path | child_process-test.js:70:9:70:49 | cmd |
153162
| child_process-test.js:70:25:70:31 | req.url | child_process-test.js:70:15:70:38 | url.par ... , true) |
154163
| child_process-test.js:70:25:70:31 | req.url | child_process-test.js:70:15:70:38 | url.par ... , true) |
164+
| child_process-test.js:80:19:80:36 | req.query.fileName | child_process-test.js:80:19:80:36 | req.query.fileName |
165+
| child_process-test.js:82:37:82:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
166+
| child_process-test.js:82:37:82:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
155167
| execSeries.js:3:20:3:22 | arr | execSeries.js:6:14:6:16 | arr |
156168
| execSeries.js:6:14:6:16 | arr | execSeries.js:6:14:6:21 | arr[i++] |
157169
| execSeries.js:6:14:6:21 | arr[i++] | execSeries.js:14:24:14:30 | command |
@@ -168,6 +180,9 @@ edges
168180
| execSeries.js:18:34:18:40 | req.url | execSeries.js:18:13:18:47 | require ... , true) |
169181
| execSeries.js:19:12:19:16 | [cmd] | execSeries.js:13:19:13:26 | commands |
170182
| execSeries.js:19:13:19:15 | cmd | execSeries.js:19:12:19:16 | [cmd] |
183+
| lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name |
184+
| lib/subLib/index.js:8:22:8:25 | name | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
185+
| lib/subLib/index.js:8:22:8:25 | name | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
171186
| other.js:5:9:5:49 | cmd | other.js:7:33:7:35 | cmd |
172187
| other.js:5:9:5:49 | cmd | other.js:7:33:7:35 | cmd |
173188
| other.js:5:9:5:49 | cmd | other.js:8:28:8:30 | cmd |
@@ -228,7 +243,9 @@ edges
228243
| child_process-test.js:59:5:59:39 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:50:15:50:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
229244
| child_process-test.js:64:3:64:21 | cp.spawn(cmd, args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:43:15:43:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
230245
| child_process-test.js:72:29:72:31 | cmd | child_process-test.js:70:25:70:31 | req.url | child_process-test.js:72:29:72:31 | cmd | This command depends on $@. | child_process-test.js:70:25:70:31 | req.url | a user-provided value |
246+
| child_process-test.js:80:19:80:36 | req.query.fileName | child_process-test.js:80:19:80:36 | req.query.fileName | child_process-test.js:80:19:80:36 | req.query.fileName | This command depends on $@. | child_process-test.js:80:19:80:36 | req.query.fileName | a user-provided value |
231247
| execSeries.js:14:41:14:47 | command | execSeries.js:18:34:18:40 | req.url | execSeries.js:14:41:14:47 | command | This command depends on $@. | execSeries.js:18:34:18:40 | req.url | a user-provided value |
248+
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | child_process-test.js:82:37:82:54 | req.query.fileName | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | This command depends on $@. | child_process-test.js:82:37:82:54 | req.query.fileName | a user-provided value |
232249
| other.js:7:33:7:35 | cmd | other.js:5:25:5:31 | req.url | other.js:7:33:7:35 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
233250
| other.js:8:28:8:30 | cmd | other.js:5:25:5:31 | req.url | other.js:8:28:8:30 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
234251
| other.js:9:32:9:34 | cmd | other.js:5:25:5:31 | req.url | other.js:9:32:9:34 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-078/child_process-test.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,14 @@ http.createServer(function(req, res) {
7272
util.promisify(cp.exec)(cmd); // NOT OK
7373
});
7474

75+
76+
const webpackDevServer = require('webpack-dev-server');
77+
new webpackDevServer(compiler, {
78+
before: function (app) {
79+
app.use(function (req, res, next) {
80+
cp.exec(req.query.fileName); // NOT OK
81+
82+
require("my-sub-lib").foo(req.query.fileName); // calls lib/subLib/index.js#foo
83+
});
84+
}
85+
});

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib/index.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,8 @@ var cp = require("child_process")
22

33
module.exports = function (name) {
44
cp.exec("rm -rf " + name); // OK - this file belongs in a sub-"module", and is not the primary exported module.
5+
};
6+
7+
module.exports.foo = function (name) {
8+
cp.exec("rm -rf " + name); // NOT OK - this is being called explicitly from child_process-test.js
59
};

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"name": "mySubLib",
2+
"name": "my-sub-lib",
33
"version": "0.0.7",
44
"main": "./index.js"
55
}

0 commit comments

Comments
 (0)