Skip to content

Commit 7790ac4

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #1409 from esben-semmle/js/more-command-injection
Approved by xiemaisi
2 parents dbf085a + 299d4c6 commit 7790ac4

File tree

5 files changed

+135
-20
lines changed

5 files changed

+135
-20
lines changed

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Improvements to JavaScript analysis
2+
3+
## General improvements
4+
5+
* Support for the following frameworks and libraries has been improved:
6+
- [cross-spawn](https://www.npmjs.com/package/cross-spawn)
7+
- [cross-spawn-async](https://www.npmjs.com/package/cross-spawn-async)
8+
- [exec](https://www.npmjs.com/package/exec)
9+
- [execa](https://www.npmjs.com/package/execa)
10+
- [exec-async](https://www.npmjs.com/package/exec-async)
11+
- [remote-exec](https://www.npmjs.com/package/remote-exec)
12+
13+
## New queries
14+
15+
| **Query** | **Tags** | **Purpose** |
16+
|-----------|----------|-------------|
17+
| | | |
18+
19+
## Changes to existing queries
20+
21+
| **Query** | **Expected impact** | **Change** |
22+
|--------------------------------|------------------------------|---------------------------------------------------------------------------|
23+
24+
25+
## Changes to QL libraries
26+

‎javascript/ql/src/javascript.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ import semmle.javascript.frameworks.React
8484
import semmle.javascript.frameworks.ReactNative
8585
import semmle.javascript.frameworks.Request
8686
import semmle.javascript.frameworks.ShellJS
87+
import semmle.javascript.frameworks.SystemCommandExecutors
8788
import semmle.javascript.frameworks.SQL
8889
import semmle.javascript.frameworks.SocketIO
8990
import semmle.javascript.frameworks.StringFormatters

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Provides concrete classes for data-flow nodes that execute an
3+
* operating system command, for instance by spawning a new process.
4+
*/
5+
6+
import javascript
7+
8+
private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::InvokeNode {
9+
int cmdArg;
10+
11+
SystemCommandExecutors() {
12+
exists(string mod, DataFlow::SourceNode callee |
13+
exists(string method |
14+
mod = "cross-spawn" and method = "sync" and cmdArg = 0
15+
or
16+
mod = "execa" and
17+
(
18+
method = "shell" or
19+
method = "shellSync" or
20+
method = "stdout" or
21+
method = "stderr" or
22+
method = "sync"
23+
) and
24+
cmdArg = 0
25+
|
26+
callee = DataFlow::moduleMember(mod, method)
27+
)
28+
or
29+
(
30+
mod = "cross-spawn" and cmdArg = 0
31+
or
32+
mod = "cross-spawn-async" and cmdArg = 0
33+
or
34+
mod = "exec" and cmdArg = 0
35+
or
36+
mod = "exec-async" and cmdArg = 0
37+
or
38+
mod = "execa" and cmdArg = 0
39+
or
40+
mod = "remote-exec" and cmdArg = 1
41+
) and
42+
callee = DataFlow::moduleImport(mod)
43+
|
44+
this = callee.getACall()
45+
)
46+
}
47+
48+
override DataFlow::Node getACommandArgument() { result = getArgument(cmdArg) }
49+
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,23 @@ nodes
5050
| execSeries.js:18:34:18:40 | req.url |
5151
| execSeries.js:19:12:19:16 | [cmd] |
5252
| execSeries.js:19:13:19:15 | cmd |
53+
| other.js:5:9:5:49 | cmd |
54+
| other.js:5:15:5:38 | url.par ... , true) |
55+
| other.js:5:15:5:44 | url.par ... ).query |
56+
| other.js:5:15:5:49 | url.par ... ry.path |
57+
| other.js:5:25:5:31 | req.url |
58+
| other.js:7:33:7:35 | cmd |
59+
| other.js:8:28:8:30 | cmd |
60+
| other.js:9:32:9:34 | cmd |
61+
| other.js:10:29:10:31 | cmd |
62+
| other.js:11:29:11:31 | cmd |
63+
| other.js:12:27:12:29 | cmd |
64+
| other.js:14:28:14:30 | cmd |
65+
| other.js:15:34:15:36 | cmd |
66+
| other.js:16:21:16:23 | cmd |
67+
| other.js:17:27:17:29 | cmd |
68+
| other.js:18:22:18:24 | cmd |
69+
| other.js:19:36:19:38 | cmd |
5370
edges
5471
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:17:13:17:15 | cmd |
5572
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:18:17:18:19 | cmd |
@@ -101,6 +118,22 @@ edges
101118
| execSeries.js:18:34:18:40 | req.url | execSeries.js:18:13:18:47 | require ... , true) |
102119
| execSeries.js:19:12:19:16 | [cmd] | execSeries.js:13:19:13:26 | commands |
103120
| execSeries.js:19:13:19:15 | cmd | execSeries.js:19:12:19:16 | [cmd] |
121+
| other.js:5:9:5:49 | cmd | other.js:7:33:7:35 | cmd |
122+
| other.js:5:9:5:49 | cmd | other.js:8:28:8:30 | cmd |
123+
| other.js:5:9:5:49 | cmd | other.js:9:32:9:34 | cmd |
124+
| other.js:5:9:5:49 | cmd | other.js:10:29:10:31 | cmd |
125+
| other.js:5:9:5:49 | cmd | other.js:11:29:11:31 | cmd |
126+
| other.js:5:9:5:49 | cmd | other.js:12:27:12:29 | cmd |
127+
| other.js:5:9:5:49 | cmd | other.js:14:28:14:30 | cmd |
128+
| other.js:5:9:5:49 | cmd | other.js:15:34:15:36 | cmd |
129+
| other.js:5:9:5:49 | cmd | other.js:16:21:16:23 | cmd |
130+
| other.js:5:9:5:49 | cmd | other.js:17:27:17:29 | cmd |
131+
| other.js:5:9:5:49 | cmd | other.js:18:22:18:24 | cmd |
132+
| other.js:5:9:5:49 | cmd | other.js:19:36:19:38 | cmd |
133+
| other.js:5:15:5:38 | url.par ... , true) | other.js:5:15:5:44 | url.par ... ).query |
134+
| other.js:5:15:5:44 | url.par ... ).query | other.js:5:15:5:49 | url.par ... ry.path |
135+
| other.js:5:15:5:49 | url.par ... ry.path | other.js:5:9:5:49 | cmd |
136+
| other.js:5:25:5:31 | req.url | other.js:5:15:5:38 | url.par ... , true) |
104137
#select
105138
| child_process-test.js:17:13:17:15 | cmd | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:17:13:17:15 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
106139
| child_process-test.js:18:17:18:19 | cmd | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:18:17:18:19 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
@@ -115,3 +148,15 @@ edges
115148
| child_process-test.js:51:5:51: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 |
116149
| child_process-test.js:56:3:56: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 |
117150
| 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 |
151+
| 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 |
152+
| 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 |
153+
| 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 |
154+
| other.js:10:29:10:31 | cmd | other.js:5:25:5:31 | req.url | other.js:10:29:10:31 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
155+
| other.js:11:29:11:31 | cmd | other.js:5:25:5:31 | req.url | other.js:11:29:11:31 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
156+
| other.js:12:27:12:29 | cmd | other.js:5:25:5:31 | req.url | other.js:12:27:12:29 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
157+
| other.js:14:28:14:30 | cmd | other.js:5:25:5:31 | req.url | other.js:14:28:14:30 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
158+
| other.js:15:34:15:36 | cmd | other.js:5:25:5:31 | req.url | other.js:15:34:15:36 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
159+
| other.js:16:21:16:23 | cmd | other.js:5:25:5:31 | req.url | other.js:16:21:16:23 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
160+
| other.js:17:27:17:29 | cmd | other.js:5:25:5:31 | req.url | other.js:17:27:17:29 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
161+
| other.js:18:22:18:24 | cmd | other.js:5:25:5:31 | req.url | other.js:18:22:18:24 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
162+
| other.js:19:36:19:38 | cmd | other.js:5:25:5:31 | req.url | other.js:19:36:19:38 | 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/other.js‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,20 @@
1-
var http =require('http'),
2-
url = require('url');
1+
var http = require("http"),
2+
url = require("url");
33

44
var server = http.createServer(function(req, res) {
55
let cmd = url.parse(req.url, true).query.path;
66

7-
var exec = require('exec');
7+
require("cross-spawn").sync(cmd); // NOT OK
8+
require("execa").shell(cmd); // NOT OK
9+
require("execa").shellSync(cmd); // NOT OK
10+
require("execa").stdout(cmd); // NOT OK
11+
require("execa").stderr(cmd); // NOT OK
12+
require("execa").sync(cmd); // NOT OK
813

9-
exec('foo'); // OK
10-
require('exec')('foo'); // OK
11-
require('exec-async').someFunction('foo'); // OK
12-
require('spawn-async').someFunction('foo'); // OK
13-
require('shelljs').someFunction('foo'); // OK
14-
require('remote-exec').someFunction('foo'); // OK
15-
require('cross-spawn').someFunction('foo'); // OK
16-
17-
18-
// NB :: we do not identify the following as sinks yet!
19-
exec(cmd); // OK (for now)
20-
require('exec')(cmd); // OK (for now)
21-
require('exec-async').someFunction(cmd); // OK (for now)
22-
require('spawn-async').someFunction(cmd); // OK (for now)
23-
require('shelljs').someFunction(cmd); // OK (for now)
24-
require('remote-exec').someFunction(cmd); // OK (for now)
25-
require('cross-spawn').someFunction(cmd); // OK (for now)
14+
require("cross-spawn")(cmd); // NOT OK
15+
require("cross-spawn-async")(cmd); // NOT OK
16+
require("exec")(cmd); // NOT OK
17+
require("exec-async")(cmd); // NOT OK
18+
require("execa")(cmd); // NOT OK
19+
require("remote-exec")(target, cmd); // NOT OK
2620
});

0 commit comments

Comments
 (0)