Skip to content

Commit 4cbfbfe

Browse files
erik-krogherik-krogh
committed
add call-edge for dynamic dispatch to unknown property from an object literal
1 parent 87461fe commit 4cbfbfe

File tree

7 files changed

+66
-0
lines changed

7 files changed

+66
-0
lines changed

‎javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,18 @@ module CallGraph {
8989
result = getAFunctionReference(outer, 0, t.continue()).getAnInvocation() and
9090
locallyReturnedFunction(outer, function)
9191
)
92+
or
93+
// dynamic dispatch to unknown property of an object
94+
exists(DataFlow::ObjectLiteralNode obj, DataFlow::PropRead read |
95+
getAFunctionReference(function, 0, t.continue()) = obj.getAPropertySource() and
96+
obj.getAPropertyRead() = read and
97+
not exists(read.getPropertyName()) and
98+
result = read and
99+
// there exists only local reads of the object, nothing else.
100+
forex(DataFlow::Node ref | ref = obj.getALocalUse() and exists(ref.asExpr()) |
101+
ref = [obj, any(DataFlow::PropRead r).getBase()]
102+
)
103+
)
92104
}
93105

94106
private predicate locallyReturnedFunction(

‎javascript/ql/test/library-tests/frameworks/NodeJSLib/tests.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ test_RouteHandler_getAResponseHeader
2727
| src/http.js:12:19:16:1 | functio ... ar");\\n} | content-type | src/http.js:13:3:13:44 | res.set ... /html') |
2828
| src/https.js:4:33:10:1 | functio ... .foo;\\n} | location | src/https.js:7:3:7:42 | res.wri ... rget }) |
2929
| src/https.js:12:20:16:1 | functio ... ar");\\n} | content-type | src/https.js:13:3:13:44 | res.set ... /html') |
30+
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | content-type | src/indirect2.js:14:3:14:51 | res.set ... /json') |
3031
| src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} | content-type | src/indirect2.js:14:3:14:51 | res.set ... /json') |
3132
test_HeaderDefinition_defines
3233
| src/http.js:13:3:13:44 | res.set ... /html') | content-type | text/html |
@@ -70,8 +71,11 @@ test_ResponseExpr
7071
| src/https.js:15:3:15:5 | res | src/https.js:12:20:16:1 | functio ... ar");\\n} |
7172
| src/indirect2.js:9:19:9:21 | res | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
7273
| src/indirect2.js:10:47:10:49 | res | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
74+
| src/indirect2.js:13:33:13:35 | res | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
7375
| src/indirect2.js:13:33:13:35 | res | src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} |
76+
| src/indirect2.js:14:3:14:5 | res | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
7477
| src/indirect2.js:14:3:14:5 | res | src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} |
78+
| src/indirect2.js:15:3:15:5 | res | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
7579
| src/indirect2.js:15:3:15:5 | res | src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} |
7680
| src/indirect.js:16:26:16:28 | res | src/indirect.js:16:12:20:5 | functio ... ;\\n } |
7781
| src/indirect.js:19:38:19:40 | res | src/indirect.js:16:12:20:5 | functio ... ;\\n } |
@@ -85,6 +89,7 @@ test_HeaderDefinition
8589
| src/http.js:63:3:63:40 | res.set ... , "23") | src/http.js:62:19:65:1 | functio ... r2");\\n} |
8690
| src/https.js:7:3:7:42 | res.wri ... rget }) | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
8791
| src/https.js:13:3:13:44 | res.set ... /html') | src/https.js:12:20:16:1 | functio ... ar");\\n} |
92+
| src/indirect2.js:14:3:14:51 | res.set ... /json') | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
8893
| src/indirect2.js:14:3:14:51 | res.set ... /json') | src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} |
8994
test_RouteSetup_getServer
9095
| createServer.js:2:1:2:42 | https.c ... es) {}) | createServer.js:2:1:2:42 | https.c ... es) {}) |
@@ -171,6 +176,9 @@ test_RouteHandler_getAResponseExpr
171176
| src/https.js:12:20:16:1 | functio ... ar");\\n} | src/https.js:15:3:15:5 | res |
172177
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:9:19:9:21 | res |
173178
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:10:47:10:49 | res |
179+
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:13:33:13:35 | res |
180+
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:14:3:14:5 | res |
181+
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:15:3:15:5 | res |
174182
| src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} | src/indirect2.js:13:33:13:35 | res |
175183
| src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} | src/indirect2.js:14:3:14:5 | res |
176184
| src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} | src/indirect2.js:15:3:15:5 | res |
@@ -307,6 +315,7 @@ test_RequestExpr
307315
| src/indirect2.js:9:14:9:16 | req | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
308316
| src/indirect2.js:10:12:10:14 | req | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
309317
| src/indirect2.js:10:42:10:44 | req | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
318+
| src/indirect2.js:13:28:13:30 | req | src/indirect2.js:9:1:11:1 | functio ... res);\\n} |
310319
| src/indirect2.js:13:28:13:30 | req | src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} |
311320
| src/indirect.js:16:21:16:23 | req | src/indirect.js:16:12:20:5 | functio ... ;\\n } |
312321
| src/indirect.js:17:28:17:30 | req | src/indirect.js:16:12:20:5 | functio ... ;\\n } |
@@ -349,6 +358,7 @@ test_RouteHandler_getARequestExpr
349358
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:9:14:9:16 | req |
350359
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:10:12:10:14 | req |
351360
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:10:42:10:44 | req |
361+
| src/indirect2.js:9:1:11:1 | functio ... res);\\n} | src/indirect2.js:13:28:13:30 | req |
352362
| src/indirect2.js:13:1:16:1 | functio ... \\"");\\n} | src/indirect2.js:13:28:13:30 | req |
353363
| src/indirect.js:16:12:20:5 | functio ... ;\\n } | src/indirect.js:16:21:16:23 | req |
354364
| src/indirect.js:16:12:20:5 | functio ... ;\\n } | src/indirect.js:17:28:17:30 | req |

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ nodes
134134
| form-parsers.js:59:10:59:33 | "touch ... ilename |
135135
| form-parsers.js:59:21:59:24 | part |
136136
| form-parsers.js:59:21:59:33 | part.filename |
137+
| lib/subLib4/index.js:6:32:6:35 | name |
138+
| lib/subLib4/index.js:7:18:7:21 | name |
139+
| lib/subLib4/subsub.js:3:28:3:31 | name |
140+
| lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name |
141+
| lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name |
142+
| lib/subLib4/subsub.js:4:22:4:25 | name |
137143
| lib/subLib/index.js:7:32:7:35 | name |
138144
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
139145
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
@@ -232,6 +238,8 @@ edges
232238
| child_process-test.js:73:25:73:31 | req.url | child_process-test.js:73:15:73:38 | url.par ... , true) |
233239
| child_process-test.js:73:25:73:31 | req.url | child_process-test.js:73:15:73:38 | url.par ... , true) |
234240
| child_process-test.js:83:19:83:36 | req.query.fileName | child_process-test.js:83:19:83:36 | req.query.fileName |
241+
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib4/index.js:6:32:6:35 | name |
242+
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib4/index.js:6:32:6:35 | name |
235243
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
236244
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
237245
| child_process-test.js:94:21:94:30 | ctx.params | child_process-test.js:94:21:94:35 | ctx.params.host |
@@ -306,6 +314,11 @@ edges
306314
| form-parsers.js:59:21:59:24 | part | form-parsers.js:59:21:59:33 | part.filename |
307315
| form-parsers.js:59:21:59:33 | part.filename | form-parsers.js:59:10:59:33 | "touch ... ilename |
308316
| form-parsers.js:59:21:59:33 | part.filename | form-parsers.js:59:10:59:33 | "touch ... ilename |
317+
| lib/subLib4/index.js:6:32:6:35 | name | lib/subLib4/index.js:7:18:7:21 | name |
318+
| lib/subLib4/index.js:7:18:7:21 | name | lib/subLib4/subsub.js:3:28:3:31 | name |
319+
| lib/subLib4/subsub.js:3:28:3:31 | name | lib/subLib4/subsub.js:4:22:4:25 | name |
320+
| lib/subLib4/subsub.js:4:22:4:25 | name | lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name |
321+
| lib/subLib4/subsub.js:4:22:4:25 | name | lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name |
309322
| lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name |
310323
| lib/subLib/index.js:8:22:8:25 | name | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
311324
| lib/subLib/index.js:8:22:8:25 | name | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
@@ -385,6 +398,7 @@ edges
385398
| form-parsers.js:41:10:41:31 | "touch ... ds.name | form-parsers.js:40:26:40:31 | fields | form-parsers.js:41:10:41:31 | "touch ... ds.name | This command depends on $@. | form-parsers.js:40:26:40:31 | fields | a user-provided value |
386399
| form-parsers.js:53:10:53:31 | "touch ... ds.name | form-parsers.js:52:34:52:39 | fields | form-parsers.js:53:10:53:31 | "touch ... ds.name | This command depends on $@. | form-parsers.js:52:34:52:39 | fields | a user-provided value |
387400
| form-parsers.js:59:10:59:33 | "touch ... ilename | form-parsers.js:58:30:58:33 | part | form-parsers.js:59:10:59:33 | "touch ... ilename | This command depends on $@. | form-parsers.js:58:30:58:33 | part | a user-provided value |
401+
| lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name | child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name | This command depends on $@. | child_process-test.js:85:37:85:54 | req.query.fileName | a user-provided value |
388402
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | This command depends on $@. | child_process-test.js:85:37:85:54 | req.query.fileName | a user-provided value |
389403
| 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 |
390404
| 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 |

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,12 @@ nodes
284284
| lib/subLib3/my-file.ts:3:28:3:31 | name |
285285
| lib/subLib3/my-file.ts:4:22:4:25 | name |
286286
| lib/subLib3/my-file.ts:4:22:4:25 | name |
287+
| lib/subLib4/index.js:6:32:6:35 | name |
288+
| lib/subLib4/index.js:6:32:6:35 | name |
289+
| lib/subLib4/index.js:7:18:7:21 | name |
290+
| lib/subLib4/subsub.js:3:28:3:31 | name |
291+
| lib/subLib4/subsub.js:4:22:4:25 | name |
292+
| lib/subLib4/subsub.js:4:22:4:25 | name |
287293
| lib/subLib/amdSub.js:3:28:3:31 | name |
288294
| lib/subLib/amdSub.js:3:28:3:31 | name |
289295
| lib/subLib/amdSub.js:4:22:4:25 | name |
@@ -640,6 +646,11 @@ edges
640646
| lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name |
641647
| lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name |
642648
| lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name |
649+
| lib/subLib4/index.js:6:32:6:35 | name | lib/subLib4/index.js:7:18:7:21 | name |
650+
| lib/subLib4/index.js:6:32:6:35 | name | lib/subLib4/index.js:7:18:7:21 | name |
651+
| lib/subLib4/index.js:7:18:7:21 | name | lib/subLib4/subsub.js:3:28:3:31 | name |
652+
| lib/subLib4/subsub.js:3:28:3:31 | name | lib/subLib4/subsub.js:4:22:4:25 | name |
653+
| lib/subLib4/subsub.js:3:28:3:31 | name | lib/subLib4/subsub.js:4:22:4:25 | name |
643654
| lib/subLib/amdSub.js:3:28:3:31 | name | lib/subLib/amdSub.js:4:22:4:25 | name |
644655
| lib/subLib/amdSub.js:3:28:3:31 | name | lib/subLib/amdSub.js:4:22:4:25 | name |
645656
| lib/subLib/amdSub.js:3:28:3:31 | name | lib/subLib/amdSub.js:4:22:4:25 | name |
@@ -735,6 +746,7 @@ edges
735746
| lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | $@ based on $@ is later used in $@. | lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | String concatenation | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command |
736747
| lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command |
737748
| lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command |
749+
| lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name | lib/subLib4/index.js:6:32:6:35 | name | lib/subLib4/subsub.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib4/subsub.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib4/index.js:6:32:6:35 | name | library input | lib/subLib4/subsub.js:4:2:4:26 | cp.exec ... + name) | shell command |
738750
| lib/subLib/amdSub.js:4:10:4:25 | "rm -rf " + name | lib/subLib/amdSub.js:3:28:3:31 | name | lib/subLib/amdSub.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib/amdSub.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib/amdSub.js:3:28:3:31 | name | library input | lib/subLib/amdSub.js:4:2:4:26 | cp.exec ... + name) | shell command |
739751
| lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:3:28:3:31 | name | library input | lib/subLib/index.js:4:2:4:26 | cp.exec ... + name) | shell command |
740752
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:7:32:7:35 | name | library input | lib/subLib/index.js:8:2:8:26 | cp.exec ... + name) | shell command |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
const dispatch = {
2+
GET: require("./bla"),
3+
POST: require("./subsub"),
4+
};
5+
6+
module.exports.foo = function (name, type) {
7+
dispatch[type](name);
8+
};

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

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": "./index.js"
5+
}

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib4/subsub.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const cp = require("child_process")
2+
3+
module.exports = function (name) {
4+
cp.exec("rm -rf " + name); // NOT OK - functions exported as part of a submodule are also flagged.
5+
};

0 commit comments

Comments
 (0)