Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions javascript/ql/lib/change-notes/2025-06-20-execa.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The model for the `execa` library has been promoted from experimental to stable.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think many users will know what to make of this, and also we shouldn't give the impression that execa didn't have a model to begin with.

Could you instead mention some of the new endpoints we're modelling now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of this 73126fe ?

1 change: 1 addition & 0 deletions javascript/ql/lib/javascript.qll
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ import semmle.javascript.frameworks.DigitalOcean
import semmle.javascript.frameworks.DomEvents
import semmle.javascript.frameworks.Electron
import semmle.javascript.frameworks.EventEmitter
import semmle.javascript.frameworks.Execa
import semmle.javascript.frameworks.Files
import semmle.javascript.frameworks.Firebase
import semmle.javascript.frameworks.FormParsers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ module Execa {
or
this = API::moduleImport("execa").getMember("execaSync").getACall() and
isSync = true
or
this = API::moduleImport("execa").getACall() and
isSync = false
}
}

Expand Down Expand Up @@ -208,4 +211,86 @@ module Execa {
private predicate isExecaShellEnable(API::Node n) {
n.getMember("shell").asSink().asExpr().(BooleanLiteral).getValue() = "true"
}

/**
* A call to `execa.node`
*/
class ExecaNodeCall extends SystemCommandExecution, API::CallNode {
ExecaNodeCall() { this = API::moduleImport("execa").getMember("node").getACall() }

override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }

override predicate isShellInterpreted(DataFlow::Node arg) { none() }

override DataFlow::Node getArgumentList() {
result = this.getArgument(1) and
not result.asExpr() instanceof ObjectExpr
}

override predicate isSync() { none() }

override DataFlow::Node getOptionsArg() {
result = this.getLastArgument() and
result.asExpr() instanceof ObjectExpr
}
}

/**
* A call to `execa.stdout`, `execa.stderr`, or `execa.sync`
*/
class ExecaStreamCall extends SystemCommandExecution, API::CallNode {
string methodName;

ExecaStreamCall() {
methodName in ["stdout", "stderr", "sync"] and
this = API::moduleImport("execa").getMember(methodName).getACall()
}

override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }

override predicate isShellInterpreted(DataFlow::Node arg) {
arg = this.getArgument(0) and
isExecaShellEnable(this.getParameter([1, 2]))
}

override DataFlow::Node getArgumentList() {
result = this.getArgument(1) and
not result.asExpr() instanceof ObjectExpr
}

override predicate isSync() { methodName = "sync" }

override DataFlow::Node getOptionsArg() {
result = this.getLastArgument() and
result.asExpr() instanceof ObjectExpr
}
}

/**
* A call to `execa.shell` or `execa.shellSync`
*/
class ExecaShellCall extends SystemCommandExecution, API::CallNode {
boolean sync;

ExecaShellCall() {
this = API::moduleImport("execa").getMember("shell").getACall() and
sync = false
or
this = API::moduleImport("execa").getMember("shellSync").getACall() and
sync = true
}

override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }

override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getACommandArgument() }

override DataFlow::Node getArgumentList() { none() }

override predicate isSync() { sync = true }

override DataFlow::Node getOptionsArg() {
result = this.getArgument(1) and
result.asExpr() instanceof ObjectExpr
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,6 @@ private predicate execApi(
cmdArg = 0 and
shell = false and
optionsArg = -1
or
mod = "execa" and
optionsArg = -1 and
(
shell = false and
fn = ["node", "stdout", "stderr", "sync"]
or
shell = true and
fn = ["command", "commandSync", "shell", "shellSync"]
) and
cmdArg = 0
)
}

Expand All @@ -38,8 +27,6 @@ private predicate execApi(string mod, int cmdArg, int optionsArg, boolean shell)
mod = "cross-spawn-async" and cmdArg = 0 and optionsArg = -1
or
mod = "exec-async" and cmdArg = 0 and optionsArg = -1
or
mod = "execa" and cmdArg = 0 and optionsArg = -1
)
or
shell = true and
Expand Down

This file was deleted.

36 changes: 0 additions & 36 deletions javascript/ql/test/experimental/Execa/CommandInjection/tests.js

This file was deleted.

38 changes: 0 additions & 38 deletions javascript/ql/test/experimental/Execa/CommandInjection/tests.ql

This file was deleted.

This file was deleted.

34 changes: 0 additions & 34 deletions javascript/ql/test/experimental/Execa/PathInjection/tests.ql

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
Expand Down Expand Up @@ -399,6 +403,15 @@ edges
| examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | examples/TaintedPath.js:8:7:8:52 | filePath | provenance | |
| examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | provenance | Config |
| examples/TaintedPath.js:10:36:10:43 | filePath | examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | provenance | Config |
| execa.js:6:9:6:64 | filePath | execa.js:9:26:9:33 | filePath | provenance | |
| execa.js:6:9:6:64 | filePath | execa.js:12:37:12:44 | filePath | provenance | |
| execa.js:6:9:6:64 | filePath | execa.js:15:50:15:57 | filePath | provenance | |
| execa.js:6:9:6:64 | filePath | execa.js:18:62:18:69 | filePath | provenance | |
| execa.js:6:20:6:43 | url.par ... , true) | execa.js:6:20:6:49 | url.par ... ).query | provenance | Config |
| execa.js:6:20:6:49 | url.par ... ).query | execa.js:6:20:6:61 | url.par ... ePath"] | provenance | Config |
| execa.js:6:20:6:61 | url.par ... ePath"] | execa.js:6:20:6:64 | url.par ... th"][0] | provenance | Config |
| execa.js:6:20:6:64 | url.par ... th"][0] | execa.js:6:9:6:64 | filePath | provenance | |
| execa.js:6:30:6:36 | req.url | execa.js:6:20:6:43 | url.par ... , true) | provenance | Config |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath | provenance | |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath | provenance | |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath | provenance | |
Expand Down Expand Up @@ -944,6 +957,16 @@ nodes
| examples/TaintedPath.js:8:28:8:34 | req.url | semmle.label | req.url |
| examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | semmle.label | ROOT + filePath |
| examples/TaintedPath.js:10:36:10:43 | filePath | semmle.label | filePath |
| execa.js:6:9:6:64 | filePath | semmle.label | filePath |
| execa.js:6:20:6:43 | url.par ... , true) | semmle.label | url.par ... , true) |
| execa.js:6:20:6:49 | url.par ... ).query | semmle.label | url.par ... ).query |
| execa.js:6:20:6:61 | url.par ... ePath"] | semmle.label | url.par ... ePath"] |
| execa.js:6:20:6:64 | url.par ... th"][0] | semmle.label | url.par ... th"][0] |
| execa.js:6:30:6:36 | req.url | semmle.label | req.url |
| execa.js:9:26:9:33 | filePath | semmle.label | filePath |
| execa.js:12:37:12:44 | filePath | semmle.label | filePath |
| execa.js:15:50:15:57 | filePath | semmle.label | filePath |
| execa.js:18:62:18:69 | filePath | semmle.label | filePath |
| express.js:8:20:8:32 | req.query.bar | semmle.label | req.query.bar |
| handlebars.js:10:51:10:58 | filePath | semmle.label | filePath |
| handlebars.js:11:32:11:39 | filePath | semmle.label | filePath |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ import http from 'node:http'
import url from 'url'

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

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

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

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

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