Skip to content

Commit 9aa26fa

Browse files
author
Max Schaefer
committed
JavaScript: Add model for foreground-child.
>1M weekly downloads, so seems worth doing.
1 parent 2f84204 commit 9aa26fa

File tree

5 files changed

+15
-0
lines changed

5 files changed

+15
-0
lines changed

change-notes/1.25/analysis-javascript.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
- [execa](https://www.npmjs.com/package/execa)
1010
- [fancy-log](https://www.npmjs.com/package/fancy-log)
1111
- [fastify](https://www.npmjs.com/package/fastify)
12+
- [foreground-child](https://www.npmjs.com/package/foreground-child)
1213
- [fstream](https://www.npmjs.com/package/fstream)
1314
- [jGrowl](https://github.com/stanlemon/jGrowl)
1415
- [jQuery](https://jquery.com/)

javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I
6565
|
6666
this = callee.getACall()
6767
)
68+
or
69+
this = DataFlow::moduleImport("foreground-child").getACall() and
70+
cmdArg = 0 and
71+
optionsArg = 1 and
72+
shell = false and
73+
sync = true
6874
}
6975

7076
override DataFlow::Node getACommandArgument() { result = getArgument(cmdArg) }

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ nodes
111111
| other.js:26:34:26:36 | cmd |
112112
| other.js:28:27:28:29 | cmd |
113113
| other.js:28:27:28:29 | cmd |
114+
| other.js:30:33:30:35 | cmd |
115+
| other.js:30:33:30:35 | cmd |
114116
| third-party-command-injection.js:5:20:5:26 | command |
115117
| third-party-command-injection.js:5:20:5:26 | command |
116118
| third-party-command-injection.js:6:21:6:27 | command |
@@ -217,6 +219,8 @@ edges
217219
| other.js:5:9:5:49 | cmd | other.js:26:34:26:36 | cmd |
218220
| other.js:5:9:5:49 | cmd | other.js:28:27:28:29 | cmd |
219221
| other.js:5:9:5:49 | cmd | other.js:28:27:28:29 | cmd |
222+
| other.js:5:9:5:49 | cmd | other.js:30:33:30:35 | cmd |
223+
| other.js:5:9:5:49 | cmd | other.js:30:33:30:35 | cmd |
220224
| other.js:5:15:5:38 | url.par ... , true) | other.js:5:15:5:44 | url.par ... ).query |
221225
| other.js:5:15:5:44 | url.par ... ).query | other.js:5:15:5:49 | url.par ... ry.path |
222226
| other.js:5:15:5:49 | url.par ... ry.path | other.js:5:9:5:49 | cmd |
@@ -266,4 +270,5 @@ edges
266270
| other.js:23:28:23:30 | cmd | other.js:5:25:5:31 | req.url | other.js:23:28:23:30 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
267271
| other.js:26:34:26:36 | cmd | other.js:5:25:5:31 | req.url | other.js:26:34:26:36 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
268272
| other.js:28:27:28:29 | cmd | other.js:5:25:5:31 | req.url | other.js:28:27:28:29 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
273+
| other.js:30:33:30:35 | cmd | other.js:5:25:5:31 | req.url | other.js:30:33:30:35 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
269274
| third-party-command-injection.js:6:21:6:27 | command | third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command | This command depends on $@. | third-party-command-injection.js:5:20:5:26 | command | a server-provided value |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ syncCommand
5656
| other.js:7:5:7:36 | require ... nc(cmd) |
5757
| other.js:9:5:9:35 | require ... nc(cmd) |
5858
| other.js:12:5:12:30 | require ... nc(cmd) |
59+
| other.js:30:5:30:36 | require ... ")(cmd) |
5960
| third-party-command-injection.js:6:9:6:28 | cp.execSync(command) |
6061
| tst_shell-command-injection-from-environment.js:4:2:4:62 | cp.exec ... emp")]) |
6162
| tst_shell-command-injection-from-environment.js:5:2:5:54 | cp.exec ... temp")) |

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,6 @@ var server = http.createServer(function(req, res) {
2626
new SSH2Stream().exec(false, cmd); // NOT OK
2727

2828
require("execa").node(cmd); // NOT OK
29+
30+
require("foreground-child")(cmd); // NOT OK
2931
});

0 commit comments

Comments
 (0)