Skip to content

Commit 7c6b4d7

Browse files
codeql-cicodeql-ci
authored
Merge pull request #4865 from esbena/js/fix-execa-model
Approved by erik-krogh
2 parents 67d0f4d + 009527c commit 7c6b4d7

File tree

5 files changed

+48
-16
lines changed

5 files changed

+48
-16
lines changed

‎javascript/change-notes/202020-12-22-execa.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* The command injection security queries now recognize additional sinks.
3+
Affected packages are
4+
[execa](https://npmjs.com/package/execa)

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,18 @@ private predicate execApi(string mod, string fn, int cmdArg, int optionsArg, boo
1818
shell = false and
1919
(
2020
fn = "node" or
21-
fn = "shell" or
22-
fn = "shellSync" or
2321
fn = "stdout" or
2422
fn = "stderr" or
2523
fn = "sync"
2624
)
2725
or
2826
shell = true and
29-
(fn = "command" or fn = "commandSync")
27+
(
28+
fn = "command" or
29+
fn = "commandSync" or
30+
fn = "shell" or
31+
fn = "shellSync"
32+
)
3033
) and
3134
cmdArg = 0
3235
}

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

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,33 @@
11
nodes
2-
| tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") |
3-
| tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") |
4-
| tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") |
5-
| tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname |
6-
| tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname |
2+
| tst_shell-command-injection-from-environment.js:6:14:6:53 | 'rm -rf ... "temp") |
3+
| tst_shell-command-injection-from-environment.js:6:14:6:53 | 'rm -rf ... "temp") |
4+
| tst_shell-command-injection-from-environment.js:6:26:6:53 | path.jo ... "temp") |
5+
| tst_shell-command-injection-from-environment.js:6:36:6:44 | __dirname |
6+
| tst_shell-command-injection-from-environment.js:6:36:6:44 | __dirname |
7+
| tst_shell-command-injection-from-environment.js:8:14:8:53 | 'rm -rf ... "temp") |
8+
| tst_shell-command-injection-from-environment.js:8:14:8:53 | 'rm -rf ... "temp") |
9+
| tst_shell-command-injection-from-environment.js:8:26:8:53 | path.jo ... "temp") |
10+
| tst_shell-command-injection-from-environment.js:8:36:8:44 | __dirname |
11+
| tst_shell-command-injection-from-environment.js:8:36:8:44 | __dirname |
12+
| tst_shell-command-injection-from-environment.js:9:18:9:57 | 'rm -rf ... "temp") |
13+
| tst_shell-command-injection-from-environment.js:9:18:9:57 | 'rm -rf ... "temp") |
14+
| tst_shell-command-injection-from-environment.js:9:30:9:57 | path.jo ... "temp") |
15+
| tst_shell-command-injection-from-environment.js:9:40:9:48 | __dirname |
16+
| tst_shell-command-injection-from-environment.js:9:40:9:48 | __dirname |
717
edges
8-
| tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") |
9-
| tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") |
10-
| tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") |
11-
| tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | tst_shell-command-injection-from-environment.js:5:26:5:53 | path.jo ... "temp") |
18+
| tst_shell-command-injection-from-environment.js:6:26:6:53 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:6:14:6:53 | 'rm -rf ... "temp") |
19+
| tst_shell-command-injection-from-environment.js:6:26:6:53 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:6:14:6:53 | 'rm -rf ... "temp") |
20+
| tst_shell-command-injection-from-environment.js:6:36:6:44 | __dirname | tst_shell-command-injection-from-environment.js:6:26:6:53 | path.jo ... "temp") |
21+
| tst_shell-command-injection-from-environment.js:6:36:6:44 | __dirname | tst_shell-command-injection-from-environment.js:6:26:6:53 | path.jo ... "temp") |
22+
| tst_shell-command-injection-from-environment.js:8:26:8:53 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:8:14:8:53 | 'rm -rf ... "temp") |
23+
| tst_shell-command-injection-from-environment.js:8:26:8:53 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:8:14:8:53 | 'rm -rf ... "temp") |
24+
| tst_shell-command-injection-from-environment.js:8:36:8:44 | __dirname | tst_shell-command-injection-from-environment.js:8:26:8:53 | path.jo ... "temp") |
25+
| tst_shell-command-injection-from-environment.js:8:36:8:44 | __dirname | tst_shell-command-injection-from-environment.js:8:26:8:53 | path.jo ... "temp") |
26+
| tst_shell-command-injection-from-environment.js:9:30:9:57 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:9:18:9:57 | 'rm -rf ... "temp") |
27+
| tst_shell-command-injection-from-environment.js:9:30:9:57 | path.jo ... "temp") | tst_shell-command-injection-from-environment.js:9:18:9:57 | 'rm -rf ... "temp") |
28+
| tst_shell-command-injection-from-environment.js:9:40:9:48 | __dirname | tst_shell-command-injection-from-environment.js:9:30:9:57 | path.jo ... "temp") |
29+
| tst_shell-command-injection-from-environment.js:9:40:9:48 | __dirname | tst_shell-command-injection-from-environment.js:9:30:9:57 | path.jo ... "temp") |
1230
#select
13-
| tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") | tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | tst_shell-command-injection-from-environment.js:5:14:5:53 | 'rm -rf ... "temp") | This shell command depends on an uncontrolled $@. | tst_shell-command-injection-from-environment.js:5:36:5:44 | __dirname | absolute path |
31+
| tst_shell-command-injection-from-environment.js:6:14:6:53 | 'rm -rf ... "temp") | tst_shell-command-injection-from-environment.js:6:36:6:44 | __dirname | tst_shell-command-injection-from-environment.js:6:14:6:53 | 'rm -rf ... "temp") | This shell command depends on an uncontrolled $@. | tst_shell-command-injection-from-environment.js:6:36:6:44 | __dirname | absolute path |
32+
| tst_shell-command-injection-from-environment.js:8:14:8:53 | 'rm -rf ... "temp") | tst_shell-command-injection-from-environment.js:8:36:8:44 | __dirname | tst_shell-command-injection-from-environment.js:8:14:8:53 | 'rm -rf ... "temp") | This shell command depends on an uncontrolled $@. | tst_shell-command-injection-from-environment.js:8:36:8:44 | __dirname | absolute path |
33+
| tst_shell-command-injection-from-environment.js:9:18:9:57 | 'rm -rf ... "temp") | tst_shell-command-injection-from-environment.js:9:40:9:48 | __dirname | tst_shell-command-injection-from-environment.js:9:18:9:57 | 'rm -rf ... "temp") | This shell command depends on an uncontrolled $@. | tst_shell-command-injection-from-environment.js:9:40:9:48 | __dirname | absolute path |

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ syncCommand
5858
| other.js:12:5:12:30 | require ... nc(cmd) |
5959
| other.js:30:5:30:36 | require ... ")(cmd) |
6060
| third-party-command-injection.js:6:9:6:28 | cp.execSync(command) |
61-
| tst_shell-command-injection-from-environment.js:4:2:4:62 | cp.exec ... emp")]) |
62-
| tst_shell-command-injection-from-environment.js:5:2:5:54 | cp.exec ... temp")) |
61+
| tst_shell-command-injection-from-environment.js:5:2:5:62 | cp.exec ... emp")]) |
62+
| tst_shell-command-injection-from-environment.js:6:2:6:54 | cp.exec ... temp")) |
63+
| tst_shell-command-injection-from-environment.js:9:2:9:58 | execa.s ... temp")) |
6364
| uselesscat.js:16:1:16:29 | execSyn ... uinfo') |
6465
| uselesscat.js:18:1:18:26 | execSyn ... path}`) |
6566
| uselesscat.js:20:1:20:36 | execSyn ... wc -l') |

‎javascript/ql/test/query-tests/Security/CWE-078/tst_shell-command-injection-from-environment.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
var cp = require('child_process'),
2-
path = require('path');
2+
path = require('path'),
3+
execa = require("execa");
34
(function() {
45
cp.execFileSync('rm', ['-rf', path.join(__dirname, "temp")]); // GOOD
56
cp.execSync('rm -rf ' + path.join(__dirname, "temp")); // BAD
7+
8+
execa.shell('rm -rf ' + path.join(__dirname, "temp")); // NOT OK
9+
execa.shellSync('rm -rf ' + path.join(__dirname, "temp")); // NOT OK
610
});

0 commit comments

Comments
 (0)