Skip to content

Commit 3d9e2f5

Browse files
authored
Merge pull request #19858 from Napalys/js/execa
JS: moved `execa` out of experimental
2 parents 7186ea5 + 73126fe commit 3d9e2f5

File tree

13 files changed

+275
-155
lines changed

13 files changed

+275
-155
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Enhanced modeling for the `execa` library, adding support for command execution methods `execaCommand`, `execaCommandSync`, `$`, and `$.sync`, as well as file system operations through `inputFile`, `pipeStdout`, `pipeAll`, and `pipeStderr`.

javascript/ql/lib/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ import semmle.javascript.frameworks.DigitalOcean
9292
import semmle.javascript.frameworks.DomEvents
9393
import semmle.javascript.frameworks.Electron
9494
import semmle.javascript.frameworks.EventEmitter
95+
import semmle.javascript.frameworks.Execa
9596
import semmle.javascript.frameworks.Files
9697
import semmle.javascript.frameworks.Firebase
9798
import semmle.javascript.frameworks.FormParsers

javascript/ql/src/experimental/semmle/javascript/Execa.qll renamed to javascript/ql/lib/semmle/javascript/frameworks/Execa.qll

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ module Execa {
5858
or
5959
this = API::moduleImport("execa").getMember("execaSync").getACall() and
6060
isSync = true
61+
or
62+
this = API::moduleImport("execa").getACall() and
63+
isSync = false
6164
}
6265
}
6366

@@ -208,4 +211,86 @@ module Execa {
208211
private predicate isExecaShellEnable(API::Node n) {
209212
n.getMember("shell").asSink().asExpr().(BooleanLiteral).getValue() = "true"
210213
}
214+
215+
/**
216+
* A call to `execa.node`
217+
*/
218+
class ExecaNodeCall extends SystemCommandExecution, API::CallNode {
219+
ExecaNodeCall() { this = API::moduleImport("execa").getMember("node").getACall() }
220+
221+
override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
222+
223+
override predicate isShellInterpreted(DataFlow::Node arg) { none() }
224+
225+
override DataFlow::Node getArgumentList() {
226+
result = this.getArgument(1) and
227+
not result.asExpr() instanceof ObjectExpr
228+
}
229+
230+
override predicate isSync() { none() }
231+
232+
override DataFlow::Node getOptionsArg() {
233+
result = this.getLastArgument() and
234+
result.asExpr() instanceof ObjectExpr
235+
}
236+
}
237+
238+
/**
239+
* A call to `execa.stdout`, `execa.stderr`, or `execa.sync`
240+
*/
241+
class ExecaStreamCall extends SystemCommandExecution, API::CallNode {
242+
string methodName;
243+
244+
ExecaStreamCall() {
245+
methodName in ["stdout", "stderr", "sync"] and
246+
this = API::moduleImport("execa").getMember(methodName).getACall()
247+
}
248+
249+
override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
250+
251+
override predicate isShellInterpreted(DataFlow::Node arg) {
252+
arg = this.getArgument(0) and
253+
isExecaShellEnable(this.getParameter([1, 2]))
254+
}
255+
256+
override DataFlow::Node getArgumentList() {
257+
result = this.getArgument(1) and
258+
not result.asExpr() instanceof ObjectExpr
259+
}
260+
261+
override predicate isSync() { methodName = "sync" }
262+
263+
override DataFlow::Node getOptionsArg() {
264+
result = this.getLastArgument() and
265+
result.asExpr() instanceof ObjectExpr
266+
}
267+
}
268+
269+
/**
270+
* A call to `execa.shell` or `execa.shellSync`
271+
*/
272+
class ExecaShellCall extends SystemCommandExecution, API::CallNode {
273+
boolean sync;
274+
275+
ExecaShellCall() {
276+
this = API::moduleImport("execa").getMember("shell").getACall() and
277+
sync = false
278+
or
279+
this = API::moduleImport("execa").getMember("shellSync").getACall() and
280+
sync = true
281+
}
282+
283+
override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
284+
285+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getACommandArgument() }
286+
287+
override DataFlow::Node getArgumentList() { none() }
288+
289+
override predicate isSync() { sync = true }
290+
291+
override DataFlow::Node getOptionsArg() {
292+
result = this.getArgument(1) and
293+
result.asExpr() instanceof ObjectExpr
294+
}
295+
}
211296
}

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,6 @@ private predicate execApi(
1616
cmdArg = 0 and
1717
shell = false and
1818
optionsArg = -1
19-
or
20-
mod = "execa" and
21-
optionsArg = -1 and
22-
(
23-
shell = false and
24-
fn = ["node", "stdout", "stderr", "sync"]
25-
or
26-
shell = true and
27-
fn = ["command", "commandSync", "shell", "shellSync"]
28-
) and
29-
cmdArg = 0
3019
)
3120
}
3221

@@ -38,8 +27,6 @@ private predicate execApi(string mod, int cmdArg, int optionsArg, boolean shell)
3827
mod = "cross-spawn-async" and cmdArg = 0 and optionsArg = -1
3928
or
4029
mod = "exec-async" and cmdArg = 0 and optionsArg = -1
41-
or
42-
mod = "execa" and cmdArg = 0 and optionsArg = -1
4330
)
4431
or
4532
shell = true and

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

Lines changed: 0 additions & 22 deletions
This file was deleted.

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

Lines changed: 0 additions & 36 deletions
This file was deleted.

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

Lines changed: 0 additions & 38 deletions
This file was deleted.

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

Lines changed: 0 additions & 6 deletions
This file was deleted.

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

Lines changed: 0 additions & 34 deletions
This file was deleted.

javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@
4848
| TaintedPath.js:214:29:214:42 | improperEscape | TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:214:29:214:42 | improperEscape | This path depends on a $@. | TaintedPath.js:212:24:212:30 | req.url | user-provided value |
4949
| TaintedPath.js:216:29:216:43 | improperEscape2 | TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:216:29:216:43 | improperEscape2 | This path depends on a $@. | TaintedPath.js:212:24:212:30 | req.url | user-provided value |
5050
| examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | This path depends on a $@. | examples/TaintedPath.js:8:28:8:34 | req.url | user-provided value |
51+
| execa.js:9:26:9:33 | filePath | execa.js:6:30:6:36 | req.url | execa.js:9:26:9:33 | filePath | This path depends on a $@. | execa.js:6:30:6:36 | req.url | user-provided value |
52+
| execa.js:12:37:12:44 | filePath | execa.js:6:30:6:36 | req.url | execa.js:12:37:12:44 | filePath | This path depends on a $@. | execa.js:6:30:6:36 | req.url | user-provided value |
53+
| execa.js:15:50:15:57 | filePath | execa.js:6:30:6:36 | req.url | execa.js:15:50:15:57 | filePath | This path depends on a $@. | execa.js:6:30:6:36 | req.url | user-provided value |
54+
| execa.js:18:62:18:69 | filePath | execa.js:6:30:6:36 | req.url | execa.js:18:62:18:69 | filePath | This path depends on a $@. | execa.js:6:30:6:36 | req.url | user-provided value |
5155
| express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | This path depends on a $@. | express.js:8:20:8:32 | req.query.bar | user-provided value |
5256
| handlebars.js:11:32:11:39 | filePath | handlebars.js:29:46:29:60 | req.params.path | handlebars.js:11:32:11:39 | filePath | This path depends on a $@. | handlebars.js:29:46:29:60 | req.params.path | user-provided value |
5357
| handlebars.js:15:25:15:32 | filePath | handlebars.js:43:15:43:29 | req.params.path | handlebars.js:15:25:15:32 | filePath | This path depends on a $@. | handlebars.js:43:15:43:29 | req.params.path | user-provided value |
@@ -399,6 +403,15 @@ edges
399403
| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | provenance | |
400404
| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | provenance | Config |
401405
| examples/TaintedPath.js:10:36:10:43 | filePath | examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | provenance | Config |
406+
| execa.js:6:9:6:64 | filePath | execa.js:9:26:9:33 | filePath | provenance | |
407+
| execa.js:6:9:6:64 | filePath | execa.js:12:37:12:44 | filePath | provenance | |
408+
| execa.js:6:9:6:64 | filePath | execa.js:15:50:15:57 | filePath | provenance | |
409+
| execa.js:6:9:6:64 | filePath | execa.js:18:62:18:69 | filePath | provenance | |
410+
| execa.js:6:20:6:43 | url.par ... , true) | execa.js:6:20:6:49 | url.par ... ).query | provenance | Config |
411+
| execa.js:6:20:6:49 | url.par ... ).query | execa.js:6:20:6:61 | url.par ... ePath"] | provenance | Config |
412+
| execa.js:6:20:6:61 | url.par ... ePath"] | execa.js:6:20:6:64 | url.par ... th"][0] | provenance | Config |
413+
| execa.js:6:20:6:64 | url.par ... th"][0] | execa.js:6:9:6:64 | filePath | provenance | |
414+
| execa.js:6:30:6:36 | req.url | execa.js:6:20:6:43 | url.par ... , true) | provenance | Config |
402415
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath | provenance | |
403416
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath | provenance | |
404417
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath | provenance | |
@@ -944,6 +957,16 @@ nodes
944957
| examples/TaintedPath.js:8:28:8:34 | req.url | semmle.label | req.url |
945958
| examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | semmle.label | ROOT + filePath |
946959
| examples/TaintedPath.js:10:36:10:43 | filePath | semmle.label | filePath |
960+
| execa.js:6:9:6:64 | filePath | semmle.label | filePath |
961+
| execa.js:6:20:6:43 | url.par ... , true) | semmle.label | url.par ... , true) |
962+
| execa.js:6:20:6:49 | url.par ... ).query | semmle.label | url.par ... ).query |
963+
| execa.js:6:20:6:61 | url.par ... ePath"] | semmle.label | url.par ... ePath"] |
964+
| execa.js:6:20:6:64 | url.par ... th"][0] | semmle.label | url.par ... th"][0] |
965+
| execa.js:6:30:6:36 | req.url | semmle.label | req.url |
966+
| execa.js:9:26:9:33 | filePath | semmle.label | filePath |
967+
| execa.js:12:37:12:44 | filePath | semmle.label | filePath |
968+
| execa.js:15:50:15:57 | filePath | semmle.label | filePath |
969+
| execa.js:18:62:18:69 | filePath | semmle.label | filePath |
947970
| express.js:8:20:8:32 | req.query.bar | semmle.label | req.query.bar |
948971
| handlebars.js:10:51:10:58 | filePath | semmle.label | filePath |
949972
| handlebars.js:11:32:11:39 | filePath | semmle.label | filePath |

javascript/ql/test/experimental/Execa/PathInjection/tests.js renamed to javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/execa.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,17 @@ import http from 'node:http'
33
import url from 'url'
44

55
http.createServer(async function (req, res) {
6-
let filePath = url.parse(req.url, true).query["filePath"][0];
6+
let filePath = url.parse(req.url, true).query["filePath"][0]; // $Source
77

88
// Piping to stdin from a file
9-
await $({ inputFile: filePath })`cat` // test: PathInjection
9+
await $({ inputFile: filePath })`cat` // $Alert
1010

1111
// Piping to stdin from a file
12-
await execa('cat', { inputFile: filePath }); // test: PathInjection
12+
await execa('cat', { inputFile: filePath }); // $Alert
1313

1414
// Piping Stdout to file
15-
await execa('echo', ['example3']).pipeStdout(filePath); // test: PathInjection
15+
await execa('echo', ['example3']).pipeStdout(filePath); // $Alert
1616

1717
// Piping all of command output to file
18-
await execa('echo', ['example4'], { all: true }).pipeAll(filePath); // test: PathInjection
19-
});
18+
await execa('echo', ['example4'], { all: true }).pipeAll(filePath); // $Alert
19+
});

0 commit comments

Comments
 (0)