Skip to content

Commit 68a5c1f

Browse files
erik-krogherik-krogh
committed
add code-injection sink for calls to node
1 parent 55e69d4 commit 68a5c1f

4 files changed

Lines changed: 44 additions & 58 deletions

File tree

‎javascript/ql/lib/semmle/javascript/security/dataflow/CodeInjectionCustomizations.qll‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,25 @@ module CodeInjection {
252252
}
253253
}
254254

255+
/**
256+
* A system command execution of "node", where the executed code is seen as a code injection sink.
257+
*/
258+
class NodeCallSink extends Sink {
259+
NodeCallSink() {
260+
exists(SystemCommandExecution s |
261+
s.getACommandArgument().mayHaveStringValue("node")
262+
or
263+
s.getACommandArgument() =
264+
DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead("0")
265+
|
266+
exists(DataFlow::SourceNode arr | arr = s.getArgumentList().getALocalSource() |
267+
arr.getAPropertyWrite().getRhs().mayHaveStringValue("-e") and
268+
this = arr.getAPropertyWrite().getRhs()
269+
)
270+
)
271+
}
272+
}
273+
255274
/** A sink for code injection via template injection. */
256275
abstract private class TemplateSink extends Sink {
257276
override string getMessageSuffix() {

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected‎

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,6 @@ nodes
5555
| angularjs.js:53:32:53:46 | location.search |
5656
| angularjs.js:53:32:53:46 | location.search |
5757
| angularjs.js:53:32:53:46 | location.search |
58-
| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
59-
| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
60-
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
61-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
62-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
63-
| bad-code-sanitization.js:56:7:56:47 | taint |
64-
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
65-
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
66-
| bad-code-sanitization.js:56:16:56:23 | req.body |
67-
| bad-code-sanitization.js:56:16:56:23 | req.body |
68-
| bad-code-sanitization.js:56:16:56:28 | req.body.name |
69-
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
70-
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
71-
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
72-
| bad-code-sanitization.js:58:44:58:48 | taint |
7358
| express.js:7:24:7:69 | "return ... + "];" |
7459
| express.js:7:24:7:69 | "return ... + "];" |
7560
| express.js:7:44:7:62 | req.param("wobble") |
@@ -94,6 +79,11 @@ nodes
9479
| express.js:21:19:21:48 | req.par ... ntext") |
9580
| express.js:21:19:21:48 | req.par ... ntext") |
9681
| express.js:21:19:21:48 | req.par ... ntext") |
82+
| express.js:26:9:26:35 | taint |
83+
| express.js:26:17:26:35 | req.param("wobble") |
84+
| express.js:26:17:26:35 | req.param("wobble") |
85+
| express.js:27:34:27:38 | taint |
86+
| express.js:27:34:27:38 | taint |
9787
| module.js:9:16:9:29 | req.query.code |
9888
| module.js:9:16:9:29 | req.query.code |
9989
| module.js:9:16:9:29 | req.query.code |
@@ -194,19 +184,6 @@ edges
194184
| angularjs.js:47:16:47:30 | location.search | angularjs.js:47:16:47:30 | location.search |
195185
| angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:36 | location.search |
196186
| angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:46 | location.search |
197-
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
198-
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
199-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
200-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
201-
| bad-code-sanitization.js:56:7:56:47 | taint | bad-code-sanitization.js:58:44:58:48 | taint |
202-
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] | bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
203-
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") | bad-code-sanitization.js:56:7:56:47 | taint |
204-
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
205-
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
206-
| bad-code-sanitization.js:56:16:56:28 | req.body.name | bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
207-
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
208-
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
209-
| bad-code-sanitization.js:58:44:58:48 | taint | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
210187
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
211188
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
212189
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" |
@@ -223,6 +200,10 @@ edges
223200
| express.js:17:30:17:53 | req.par ... cript") | express.js:17:30:17:53 | req.par ... cript") |
224201
| express.js:19:37:19:70 | req.par ... odule") | express.js:19:37:19:70 | req.par ... odule") |
225202
| express.js:21:19:21:48 | req.par ... ntext") | express.js:21:19:21:48 | req.par ... ntext") |
203+
| express.js:26:9:26:35 | taint | express.js:27:34:27:38 | taint |
204+
| express.js:26:9:26:35 | taint | express.js:27:34:27:38 | taint |
205+
| express.js:26:17:26:35 | req.param("wobble") | express.js:26:9:26:35 | taint |
206+
| express.js:26:17:26:35 | req.param("wobble") | express.js:26:9:26:35 | taint |
226207
| module.js:9:16:9:29 | req.query.code | module.js:9:16:9:29 | req.query.code |
227208
| module.js:11:17:11:30 | req.query.code | module.js:11:17:11:30 | req.query.code |
228209
| react-native.js:7:7:7:33 | tainted | react-native.js:8:32:8:38 | tainted |
@@ -299,15 +280,14 @@ edges
299280
| angularjs.js:47:16:47:30 | location.search | angularjs.js:47:16:47:30 | location.search | angularjs.js:47:16:47:30 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:47:16:47:30 | location.search | User-provided value |
300281
| angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:50:22:50:36 | location.search | User-provided value |
301282
| angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:46 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:53:32:53:46 | location.search | User-provided value |
302-
| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | $@ flows to here and is interpreted as code. | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | User-provided value |
303-
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` | bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` | $@ flows to here and is interpreted as code. | bad-code-sanitization.js:56:16:56:23 | req.body | User-provided value |
304283
| express.js:7:24:7:69 | "return ... + "];" | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:7:44:7:62 | req.param("wobble") | User-provided value |
305284
| express.js:9:34:9:79 | "return ... + "];" | express.js:9:54:9:72 | req.param("wobble") | express.js:9:34:9:79 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:9:54:9:72 | req.param("wobble") | User-provided value |
306285
| express.js:12:8:12:53 | "return ... + "];" | express.js:12:28:12:46 | req.param("wobble") | express.js:12:8:12:53 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:12:28:12:46 | req.param("wobble") | User-provided value |
307286
| express.js:15:22:15:54 | req.par ... ction") | express.js:15:22:15:54 | req.par ... ction") | express.js:15:22:15:54 | req.par ... ction") | $@ flows to here and is interpreted as code. | express.js:15:22:15:54 | req.par ... ction") | User-provided value |
308287
| express.js:17:30:17:53 | req.par ... cript") | express.js:17:30:17:53 | req.par ... cript") | express.js:17:30:17:53 | req.par ... cript") | $@ flows to here and is interpreted as code. | express.js:17:30:17:53 | req.par ... cript") | User-provided value |
309288
| express.js:19:37:19:70 | req.par ... odule") | express.js:19:37:19:70 | req.par ... odule") | express.js:19:37:19:70 | req.par ... odule") | $@ flows to here and is interpreted as code. | express.js:19:37:19:70 | req.par ... odule") | User-provided value |
310289
| express.js:21:19:21:48 | req.par ... ntext") | express.js:21:19:21:48 | req.par ... ntext") | express.js:21:19:21:48 | req.par ... ntext") | $@ flows to here and is interpreted as code. | express.js:21:19:21:48 | req.par ... ntext") | User-provided value |
290+
| express.js:27:34:27:38 | taint | express.js:26:17:26:35 | req.param("wobble") | express.js:27:34:27:38 | taint | $@ flows to here and is interpreted as code. | express.js:26:17:26:35 | req.param("wobble") | User-provided value |
311291
| module.js:9:16:9:29 | req.query.code | module.js:9:16:9:29 | req.query.code | module.js:9:16:9:29 | req.query.code | $@ flows to here and is interpreted as code. | module.js:9:16:9:29 | req.query.code | User-provided value |
312292
| module.js:11:17:11:30 | req.query.code | module.js:11:17:11:30 | req.query.code | module.js:11:17:11:30 | req.query.code | $@ flows to here and is interpreted as code. | module.js:11:17:11:30 | req.query.code | User-provided value |
313293
| react-native.js:8:32:8:38 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:8:32:8:38 | tainted | $@ flows to here and is interpreted as code. | react-native.js:7:17:7:33 | req.param("code") | User-provided value |

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected‎

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,6 @@ nodes
5555
| angularjs.js:53:32:53:46 | location.search |
5656
| angularjs.js:53:32:53:46 | location.search |
5757
| angularjs.js:53:32:53:46 | location.search |
58-
| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
59-
| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
60-
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
61-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
62-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") |
63-
| bad-code-sanitization.js:56:7:56:47 | taint |
64-
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
65-
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
66-
| bad-code-sanitization.js:56:16:56:23 | req.body |
67-
| bad-code-sanitization.js:56:16:56:23 | req.body |
68-
| bad-code-sanitization.js:56:16:56:28 | req.body.name |
69-
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
70-
| bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
71-
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
72-
| bad-code-sanitization.js:58:44:58:48 | taint |
7358
| eslint-escope-build.js:20:22:20:22 | c |
7459
| eslint-escope-build.js:20:22:20:22 | c |
7560
| eslint-escope-build.js:21:16:21:16 | c |
@@ -98,6 +83,11 @@ nodes
9883
| express.js:21:19:21:48 | req.par ... ntext") |
9984
| express.js:21:19:21:48 | req.par ... ntext") |
10085
| express.js:21:19:21:48 | req.par ... ntext") |
86+
| express.js:26:9:26:35 | taint |
87+
| express.js:26:17:26:35 | req.param("wobble") |
88+
| express.js:26:17:26:35 | req.param("wobble") |
89+
| express.js:27:34:27:38 | taint |
90+
| express.js:27:34:27:38 | taint |
10191
| module.js:9:16:9:29 | req.query.code |
10292
| module.js:9:16:9:29 | req.query.code |
10393
| module.js:9:16:9:29 | req.query.code |
@@ -198,19 +188,6 @@ edges
198188
| angularjs.js:47:16:47:30 | location.search | angularjs.js:47:16:47:30 | location.search |
199189
| angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:36 | location.search |
200190
| angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:46 | location.search |
201-
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
202-
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` |
203-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
204-
| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
205-
| bad-code-sanitization.js:56:7:56:47 | taint | bad-code-sanitization.js:58:44:58:48 | taint |
206-
| bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] | bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") |
207-
| bad-code-sanitization.js:56:15:56:47 | [req.bo ... n("\\n") | bad-code-sanitization.js:56:7:56:47 | taint |
208-
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
209-
| bad-code-sanitization.js:56:16:56:23 | req.body | bad-code-sanitization.js:56:16:56:28 | req.body.name |
210-
| bad-code-sanitization.js:56:16:56:28 | req.body.name | bad-code-sanitization.js:56:15:56:36 | [req.bo ... "foo"] |
211-
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
212-
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:14:58:53 | `(funct ... nt)}))` |
213-
| bad-code-sanitization.js:58:44:58:48 | taint | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
214191
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
215192
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
216193
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
@@ -231,6 +208,10 @@ edges
231208
| express.js:17:30:17:53 | req.par ... cript") | express.js:17:30:17:53 | req.par ... cript") |
232209
| express.js:19:37:19:70 | req.par ... odule") | express.js:19:37:19:70 | req.par ... odule") |
233210
| express.js:21:19:21:48 | req.par ... ntext") | express.js:21:19:21:48 | req.par ... ntext") |
211+
| express.js:26:9:26:35 | taint | express.js:27:34:27:38 | taint |
212+
| express.js:26:9:26:35 | taint | express.js:27:34:27:38 | taint |
213+
| express.js:26:17:26:35 | req.param("wobble") | express.js:26:9:26:35 | taint |
214+
| express.js:26:17:26:35 | req.param("wobble") | express.js:26:9:26:35 | taint |
234215
| module.js:9:16:9:29 | req.query.code | module.js:9:16:9:29 | req.query.code |
235216
| module.js:11:17:11:30 | req.query.code | module.js:11:17:11:30 | req.query.code |
236217
| react-native.js:7:7:7:33 | tainted | react-native.js:8:32:8:38 | tainted |

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/express.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,9 @@ app.get('/some/path', function(req, res) {
2020
// NOT OK
2121
vm.runInContext(req.param("code_runInContext"), vm.createContext());
2222
});
23+
24+
const cp = require('child_process');
25+
app.get('/other/path', function(req, res) {
26+
const taint = req.param("wobble");
27+
cp.execFileSync('node', ['-e', taint]); // NOT OK
28+
});

0 commit comments

Comments
 (0)