Skip to content

Commit c743aba

Browse files
erik-krogherik-krogh
authored
Merge pull request github#14294 from am0o0/amammad-js-CodeInjection_execa
JS: provide command execution sinks for execa package
2 parents 5a7174d + c80f48b commit c743aba

File tree

7 files changed

+366
-0
lines changed

7 files changed

+366
-0
lines changed

‎javascript/ql/src/experimental/semmle/javascript/Execa.qll

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
/**
2+
* Models the `execa` library in terms of `FileSystemAccess` and `SystemCommandExecution`.
3+
*/
4+
5+
import javascript
6+
7+
/**
8+
* Provide model for [Execa](https://github.com/sindresorhus/execa) package
9+
*/
10+
module Execa {
11+
/**
12+
* The Execa input file read and output file write
13+
*/
14+
class ExecaFileSystemAccess extends FileSystemReadAccess, DataFlow::Node {
15+
API::Node execaArg;
16+
boolean isPipedToFile;
17+
18+
ExecaFileSystemAccess() {
19+
(
20+
execaArg = API::moduleImport("execa").getMember("$").getParameter(0) and
21+
isPipedToFile = false
22+
or
23+
execaArg =
24+
API::moduleImport("execa")
25+
.getMember(["execa", "execaCommand", "execaCommandSync", "execaSync"])
26+
.getParameter([0, 1, 2]) and
27+
isPipedToFile = false
28+
or
29+
execaArg =
30+
API::moduleImport("execa")
31+
.getMember(["execa", "execaCommand", "execaCommandSync", "execaSync"])
32+
.getReturn()
33+
.getMember(["pipeStdout", "pipeAll", "pipeStderr"])
34+
.getParameter(0) and
35+
isPipedToFile = true
36+
) and
37+
this = execaArg.asSink()
38+
}
39+
40+
override DataFlow::Node getADataNode() { none() }
41+
42+
override DataFlow::Node getAPathArgument() {
43+
result = execaArg.getMember("inputFile").asSink() and isPipedToFile = false
44+
or
45+
result = execaArg.asSink() and isPipedToFile = true
46+
}
47+
}
48+
49+
/**
50+
* A call to `execa.execa` or `execa.execaSync`
51+
*/
52+
class ExecaCall extends API::CallNode {
53+
boolean isSync;
54+
55+
ExecaCall() {
56+
this = API::moduleImport("execa").getMember("execa").getACall() and
57+
isSync = false
58+
or
59+
this = API::moduleImport("execa").getMember("execaSync").getACall() and
60+
isSync = true
61+
}
62+
}
63+
64+
/**
65+
* The system command execution nodes for `execa.execa` or `execa.execaSync` functions
66+
*/
67+
class ExecaExec extends SystemCommandExecution, ExecaCall {
68+
ExecaExec() { isSync = [false, true] }
69+
70+
override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
71+
72+
override predicate isShellInterpreted(DataFlow::Node arg) {
73+
// if shell: true then first and second args are sinks
74+
// options can be third argument
75+
arg = [this.getArgument(0), this.getParameter(1).getUnknownMember().asSink()] and
76+
isExecaShellEnable(this.getParameter(2))
77+
or
78+
// options can be second argument
79+
arg = this.getArgument(0) and
80+
isExecaShellEnable(this.getParameter(1))
81+
}
82+
83+
override DataFlow::Node getArgumentList() {
84+
// execa(cmd, [arg]);
85+
exists(DataFlow::Node arg | arg = this.getArgument(1) |
86+
// if it is a object then it is a option argument not command argument
87+
result = arg and not arg.asExpr() instanceof ObjectExpr
88+
)
89+
}
90+
91+
override predicate isSync() { isSync = true }
92+
93+
override DataFlow::Node getOptionsArg() {
94+
result = this.getLastArgument() and result.asExpr() instanceof ObjectExpr
95+
}
96+
}
97+
98+
/**
99+
* A call to `execa.$` or `execa.$.sync` or `execa.$({})` or `execa.$.sync({})` tag functions
100+
*/
101+
private class ExecaScriptCall extends API::CallNode {
102+
boolean isSync;
103+
104+
ExecaScriptCall() {
105+
exists(API::Node script |
106+
script =
107+
[
108+
API::moduleImport("execa").getMember("$"),
109+
API::moduleImport("execa").getMember("$").getReturn()
110+
]
111+
|
112+
this = script.getACall() and
113+
isSync = false
114+
or
115+
this = script.getMember("sync").getACall() and
116+
isSync = true
117+
)
118+
}
119+
}
120+
121+
/**
122+
* The system command execution nodes for `execa.$` or `execa.$.sync` tag functions
123+
*/
124+
class ExecaScript extends SystemCommandExecution, ExecaScriptCall {
125+
ExecaScript() { isSync = [false, true] }
126+
127+
override DataFlow::Node getACommandArgument() {
128+
result = this.getParameter(1).asSink() and
129+
not isTaggedTemplateFirstChildAnElement(this.getParameter(1).asSink().asExpr().getParent())
130+
}
131+
132+
override predicate isShellInterpreted(DataFlow::Node arg) {
133+
isExecaShellEnable(this.getParameter(0)) and
134+
arg = this.getAParameter().asSink()
135+
}
136+
137+
override DataFlow::Node getArgumentList() {
138+
result = this.getParameter(any(int i | i >= 1)).asSink() and
139+
isTaggedTemplateFirstChildAnElement(this.getParameter(1).asSink().asExpr().getParent())
140+
or
141+
result = this.getParameter(any(int i | i >= 2)).asSink() and
142+
not isTaggedTemplateFirstChildAnElement(this.getParameter(1).asSink().asExpr().getParent())
143+
}
144+
145+
override DataFlow::Node getOptionsArg() { result = this.getParameter(0).asSink() }
146+
147+
override predicate isSync() { isSync = true }
148+
}
149+
150+
/**
151+
* A call to `execa.execaCommandSync` or `execa.execaCommand`
152+
*/
153+
private class ExecaCommandCall extends API::CallNode {
154+
boolean isSync;
155+
156+
ExecaCommandCall() {
157+
this = API::moduleImport("execa").getMember("execaCommandSync").getACall() and
158+
isSync = true
159+
or
160+
this = API::moduleImport("execa").getMember("execaCommand").getACall() and
161+
isSync = false
162+
}
163+
}
164+
165+
/**
166+
* The system command execution nodes for `execa.execaCommand` or `execa.execaCommandSync` functions
167+
*/
168+
class ExecaCommandExec extends SystemCommandExecution, ExecaCommandCall {
169+
ExecaCommandExec() { isSync = [false, true] }
170+
171+
override DataFlow::Node getACommandArgument() {
172+
result = this.(DataFlow::CallNode).getArgument(0)
173+
}
174+
175+
override DataFlow::Node getArgumentList() {
176+
// execaCommand(`${cmd} ${arg}`);
177+
result.asExpr() = this.getParameter(0).asSink().asExpr().getAChildExpr() and
178+
not result.asExpr() = this.getArgument(0).asExpr().getChildExpr(0)
179+
}
180+
181+
override predicate isShellInterpreted(DataFlow::Node arg) {
182+
// execaCommandSync(`${cmd} ${arg}`, {shell: true})
183+
arg.asExpr() = this.getArgument(0).asExpr().getAChildExpr+() and
184+
isExecaShellEnable(this.getParameter(1))
185+
or
186+
// there is only one argument that is constructed in previous nodes,
187+
// it makes sanitizing really hard to select whether it is vulnerable to argument injection or not
188+
arg = this.getParameter(0).asSink() and
189+
not exists(this.getArgument(0).asExpr().getChildExpr(1))
190+
}
191+
192+
override predicate isSync() { isSync = true }
193+
194+
override DataFlow::Node getOptionsArg() {
195+
result = this.getLastArgument() and result.asExpr() instanceof ObjectExpr
196+
}
197+
}
198+
199+
/** Gets a TemplateLiteral and check if first child is a template element */
200+
private predicate isTaggedTemplateFirstChildAnElement(TemplateLiteral templateLit) {
201+
exists(templateLit.getChildExpr(0).(TemplateElement))
202+
}
203+
204+
/**
205+
* Holds whether Execa has shell enabled options or not, get Parameter responsible for options
206+
*/
207+
pragma[inline]
208+
private predicate isExecaShellEnable(API::Node n) {
209+
n.getMember("shell").asSink().asExpr().(BooleanLiteral).getValue() = "true"
210+
}
211+
}

‎javascript/ql/test/experimental/Execa/CommandInjection/tests.expected

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
passingPositiveTests
2+
| PASSED | CommandInjection | tests.js:11:46:11:70 | // test ... jection |
3+
| PASSED | CommandInjection | tests.js:12:43:12:67 | // test ... jection |
4+
| PASSED | CommandInjection | tests.js:13:63:13:87 | // test ... jection |
5+
| PASSED | CommandInjection | tests.js:14:62:14:86 | // test ... jection |
6+
| PASSED | CommandInjection | tests.js:15:60:15:84 | // test ... jection |
7+
| PASSED | CommandInjection | tests.js:17:45:17:69 | // test ... jection |
8+
| PASSED | CommandInjection | tests.js:18:42:18:66 | // test ... jection |
9+
| PASSED | CommandInjection | tests.js:19:62:19:86 | // test ... jection |
10+
| PASSED | CommandInjection | tests.js:20:63:20:87 | // test ... jection |
11+
| PASSED | CommandInjection | tests.js:21:60:21:84 | // test ... jection |
12+
| PASSED | CommandInjection | tests.js:23:43:23:67 | // test ... jection |
13+
| PASSED | CommandInjection | tests.js:24:40:24:64 | // test ... jection |
14+
| PASSED | CommandInjection | tests.js:25:40:25:64 | // test ... jection |
15+
| PASSED | CommandInjection | tests.js:26:60:26:84 | // test ... jection |
16+
| PASSED | CommandInjection | tests.js:28:41:28:65 | // test ... jection |
17+
| PASSED | CommandInjection | tests.js:29:58:29:82 | // test ... jection |
18+
| PASSED | CommandInjection | tests.js:31:51:31:75 | // test ... jection |
19+
| PASSED | CommandInjection | tests.js:32:68:32:92 | // test ... jection |
20+
| PASSED | CommandInjection | tests.js:34:49:34:73 | // test ... jection |
21+
| PASSED | CommandInjection | tests.js:35:66:35:90 | // test ... jection |
22+
failingPositiveTests

‎javascript/ql/test/experimental/Execa/CommandInjection/tests.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { execa, execaSync, execaCommand, execaCommandSync, $ } from 'execa';
2+
import http from 'node:http'
3+
import url from 'url'
4+
5+
http.createServer(async function (req, res) {
6+
let cmd = url.parse(req.url, true).query["cmd"][0];
7+
let arg1 = url.parse(req.url, true).query["arg1"];
8+
let arg2 = url.parse(req.url, true).query["arg2"];
9+
let arg3 = url.parse(req.url, true).query["arg3"];
10+
11+
await $`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
12+
await $`ssh ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
13+
$({ shell: false }).sync`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
14+
$({ shell: true }).sync`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
15+
$({ shell: false }).sync`ssh ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
16+
17+
$.sync`${cmd} ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
18+
$.sync`ssh ${arg1} ${arg2} ${arg3}`; // test: CommandInjection
19+
await $({ shell: true })`${cmd} ${arg1} ${arg2} ${arg3}` // test: CommandInjection
20+
await $({ shell: false })`${cmd} ${arg1} ${arg2} ${arg3}` // test: CommandInjection
21+
await $({ shell: false })`ssh ${arg1} ${arg2} ${arg3}` // test: CommandInjection
22+
23+
await execa(cmd, [arg1, arg2, arg3]); // test: CommandInjection
24+
await execa(cmd, { shell: true }); // test: CommandInjection
25+
await execa(cmd, { shell: true }); // test: CommandInjection
26+
await execa(cmd, [arg1, arg2, arg3], { shell: true }); // test: CommandInjection
27+
28+
execaSync(cmd, [arg1, arg2, arg3]); // test: CommandInjection
29+
execaSync(cmd, [arg1, arg2, arg3], { shell: true }); // test: CommandInjection
30+
31+
await execaCommand(cmd + arg1 + arg2 + arg3); // test: CommandInjection
32+
await execaCommand(cmd + arg1 + arg2 + arg3, { shell: true }); // test: CommandInjection
33+
34+
execaCommandSync(cmd + arg1 + arg2 + arg3); // test: CommandInjection
35+
execaCommandSync(cmd + arg1 + arg2 + arg3, { shell: true }); // test: CommandInjection
36+
});

‎javascript/ql/test/experimental/Execa/CommandInjection/tests.ql

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import javascript
2+
3+
class InlineTest extends LineComment {
4+
string tests;
5+
6+
InlineTest() { tests = this.getText().regexpCapture("\\s*test:(.*)", 1) }
7+
8+
string getPositiveTest() {
9+
result = tests.trim().splitAt(",").trim() and not result.matches("!%")
10+
}
11+
12+
predicate hasPositiveTest(string test) { test = this.getPositiveTest() }
13+
14+
predicate inNode(DataFlow::Node n) {
15+
this.getLocation().getFile() = n.getFile() and
16+
this.getLocation().getStartLine() = n.getStartLine()
17+
}
18+
}
19+
20+
import experimental.semmle.javascript.Execa
21+
22+
query predicate passingPositiveTests(string res, string expectation, InlineTest t) {
23+
res = "PASSED" and
24+
t.hasPositiveTest(expectation) and
25+
expectation = "CommandInjection" and
26+
exists(SystemCommandExecution n |
27+
t.inNode(n.getArgumentList()) or t.inNode(n.getACommandArgument())
28+
)
29+
}
30+
31+
query predicate failingPositiveTests(string res, string expectation, InlineTest t) {
32+
res = "FAILED" and
33+
t.hasPositiveTest(expectation) and
34+
expectation = "CommandInjection" and
35+
not exists(SystemCommandExecution n |
36+
t.inNode(n.getArgumentList()) or t.inNode(n.getACommandArgument())
37+
)
38+
}

‎javascript/ql/test/experimental/Execa/PathInjection/tests.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
passingPositiveTests
2+
| PASSED | PathInjection | tests.js:9:43:9:64 | // test ... jection |
3+
| PASSED | PathInjection | tests.js:12:50:12:71 | // test ... jection |
4+
| PASSED | PathInjection | tests.js:15:61:15:82 | // test ... jection |
5+
| PASSED | PathInjection | tests.js:18:73:18:94 | // test ... jection |
6+
failingPositiveTests

‎javascript/ql/test/experimental/Execa/PathInjection/tests.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { execa, $ } from 'execa';
2+
import http from 'node:http'
3+
import url from 'url'
4+
5+
http.createServer(async function (req, res) {
6+
let filePath = url.parse(req.url, true).query["filePath"][0];
7+
8+
// Piping to stdin from a file
9+
await $({ inputFile: filePath })`cat` // test: PathInjection
10+
11+
// Piping to stdin from a file
12+
await execa('cat', { inputFile: filePath }); // test: PathInjection
13+
14+
// Piping Stdout to file
15+
await execa('echo', ['example3']).pipeStdout(filePath); // test: PathInjection
16+
17+
// Piping all of command output to file
18+
await execa('echo', ['example4'], { all: true }).pipeAll(filePath); // test: PathInjection
19+
});

‎javascript/ql/test/experimental/Execa/PathInjection/tests.ql

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import javascript
2+
3+
class InlineTest extends LineComment {
4+
string tests;
5+
6+
InlineTest() { tests = this.getText().regexpCapture("\\s*test:(.*)", 1) }
7+
8+
string getPositiveTest() {
9+
result = tests.trim().splitAt(",").trim() and not result.matches("!%")
10+
}
11+
12+
predicate hasPositiveTest(string test) { test = this.getPositiveTest() }
13+
14+
predicate inNode(DataFlow::Node n) {
15+
this.getLocation().getFile() = n.getFile() and
16+
this.getLocation().getStartLine() = n.getStartLine()
17+
}
18+
}
19+
20+
import experimental.semmle.javascript.Execa
21+
22+
query predicate passingPositiveTests(string res, string expectation, InlineTest t) {
23+
res = "PASSED" and
24+
t.hasPositiveTest(expectation) and
25+
expectation = "PathInjection" and
26+
exists(FileSystemReadAccess n | t.inNode(n.getAPathArgument()))
27+
}
28+
29+
query predicate failingPositiveTests(string res, string expectation, InlineTest t) {
30+
res = "FAILED" and
31+
t.hasPositiveTest(expectation) and
32+
expectation = "PathInjection" and
33+
not exists(FileSystemReadAccess n | t.inNode(n.getAPathArgument()))
34+
}

0 commit comments

Comments
 (0)