Skip to content

Commit c376eeb

Browse files
asgerfasgerf
authored
Merge pull request github#12978 from asgerf/js/github-actions-sources
JS: Add sources and sinks related to GitHub Actions
2 parents fa0a999 + b282543 commit c376eeb

File tree

11 files changed

+220
-0
lines changed

11 files changed

+220
-0
lines changed

‎javascript/ql/lib/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import semmle.javascript.YAML
6767
import semmle.javascript.dataflow.DataFlow
6868
import semmle.javascript.dataflow.TaintTracking
6969
import semmle.javascript.dataflow.TypeInference
70+
import semmle.javascript.frameworks.ActionsLib
7071
import semmle.javascript.frameworks.Angular2
7172
import semmle.javascript.frameworks.AngularJS
7273
import semmle.javascript.frameworks.Anser

‎javascript/ql/lib/semmle/javascript/frameworks/ActionsLib.qll

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* Contains models for `@actions/core` related libraries.
3+
*/
4+
5+
private import javascript
6+
private import semmle.javascript.security.dataflow.IndirectCommandInjectionCustomizations
7+
8+
private API::Node payload() {
9+
result = API::moduleImport("@actions/github").getMember("context").getMember("payload")
10+
}
11+
12+
private API::Node workflowRun() { result = payload().getMember("workflow_run") }
13+
14+
private API::Node commitObj() {
15+
result = workflowRun().getMember("head_commit")
16+
or
17+
result = payload().getMember("commits").getAMember()
18+
}
19+
20+
private API::Node pullRequest() {
21+
result = payload().getMember("pull_request")
22+
or
23+
result = commitObj().getMember("pull_requests").getAMember()
24+
}
25+
26+
private API::Node taintSource() {
27+
result = pullRequest().getMember("head").getMember(["ref", "label"])
28+
or
29+
result =
30+
[pullRequest(), payload().getMember(["discussion", "issue"])].getMember(["title", "body"])
31+
or
32+
result = payload().getMember(["review", "review_comment", "comment"]).getMember("body")
33+
or
34+
result = workflowRun().getMember(["head_branch", "display_title"])
35+
or
36+
result = workflowRun().getMember("head_repository").getMember("description")
37+
or
38+
result = commitObj().getMember("message")
39+
or
40+
result = commitObj().getMember(["author", "committer"]).getMember(["name", "email"])
41+
}
42+
43+
/**
44+
* A source of taint originating from the context.
45+
*/
46+
private class GitHubActionsContextSource extends RemoteFlowSource {
47+
GitHubActionsContextSource() { this = taintSource().asSource() }
48+
49+
override string getSourceType() { result = "GitHub Actions context" }
50+
}
51+
52+
/**
53+
* A source of taint originating from user input.
54+
*
55+
* At the momemnt this is only treated as a taint source for the indirect-command injection
56+
* query.
57+
*/
58+
private class GitHubActionsInputSource extends IndirectCommandInjection::Source {
59+
GitHubActionsInputSource() {
60+
this =
61+
API::moduleImport("@actions/core")
62+
.getMember(["getInput", "getMultilineInput"])
63+
.getReturn()
64+
.asSource()
65+
}
66+
67+
override string describe() { result = "GitHub Actions user input" }
68+
}
69+
70+
private class ExecActionsCall extends SystemCommandExecution, DataFlow::CallNode {
71+
ExecActionsCall() {
72+
this = API::moduleImport("@actions/exec").getMember(["exec", "getExecOutput"]).getACall()
73+
}
74+
75+
override DataFlow::Node getACommandArgument() { result = this.getArgument(0) }
76+
77+
override DataFlow::Node getArgumentList() { result = this.getArgument(1) }
78+
79+
override DataFlow::Node getOptionsArg() { result = this.getArgument(2) }
80+
81+
override predicate isSync() { none() }
82+
}

‎javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,38 @@ module IndirectCommandInjection {
4949
override string describe() { result = "environment variable" }
5050
}
5151

52+
/** Gets a data flow node referring to `process.env`. */
53+
private DataFlow::SourceNode envObject(DataFlow::TypeTracker t) {
54+
t.start() and
55+
result = NodeJSLib::process().getAPropertyRead("env")
56+
or
57+
exists(DataFlow::TypeTracker t2 | result = envObject(t2).track(t2, t))
58+
}
59+
60+
/** Gets a data flow node referring to `process.env`. */
61+
private DataFlow::SourceNode envObject() { result = envObject(DataFlow::TypeTracker::end()) }
62+
63+
/**
64+
* Gets the name of an environment variable that is assumed to be safe.
65+
*/
66+
private string getASafeEnvironmentVariable() {
67+
result =
68+
[
69+
"GITHUB_ACTION", "GITHUB_ACTION_PATH", "GITHUB_ACTION_REPOSITORY", "GITHUB_ACTIONS",
70+
"GITHUB_ACTOR", "GITHUB_API_URL", "GITHUB_BASE_REF", "GITHUB_ENV", "GITHUB_EVENT_NAME",
71+
"GITHUB_EVENT_PATH", "GITHUB_GRAPHQL_URL", "GITHUB_JOB", "GITHUB_PATH", "GITHUB_REF",
72+
"GITHUB_REPOSITORY", "GITHUB_REPOSITORY_OWNER", "GITHUB_RUN_ID", "GITHUB_RUN_NUMBER",
73+
"GITHUB_SERVER_URL", "GITHUB_SHA", "GITHUB_WORKFLOW", "GITHUB_WORKSPACE"
74+
]
75+
}
76+
77+
/** Sanitizer that blocks flow through safe environment variables. */
78+
private class SafeEnvVariableSanitizer extends Sanitizer {
79+
SafeEnvVariableSanitizer() {
80+
this = envObject().getAPropertyRead(getASafeEnvironmentVariable())
81+
}
82+
}
83+
5284
/**
5385
* An object containing parsed command-line arguments, considered as a flow source for command injection.
5486
*/

‎javascript/ql/src/change-notes/2023-05-02-github-actions-sources.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* Added taint sources from the `@actions/core` and `@actions/github` packages.
5+
* Added command-injection sinks from the `@actions/exec` package.

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,16 @@
11
nodes
2+
| actions.js:8:9:8:57 | title |
3+
| actions.js:8:17:8:57 | github. ... t.title |
4+
| actions.js:8:17:8:57 | github. ... t.title |
5+
| actions.js:9:8:9:22 | `echo ${title}` |
6+
| actions.js:9:8:9:22 | `echo ${title}` |
7+
| actions.js:9:16:9:20 | title |
8+
| actions.js:18:9:18:63 | head_ref |
9+
| actions.js:18:20:18:63 | github. ... ead.ref |
10+
| actions.js:18:20:18:63 | github. ... ead.ref |
11+
| actions.js:19:14:19:31 | `echo ${head_ref}` |
12+
| actions.js:19:14:19:31 | `echo ${head_ref}` |
13+
| actions.js:19:22:19:29 | head_ref |
214
| child_process-test.js:6:9:6:49 | cmd |
315
| child_process-test.js:6:15:6:38 | url.par ... , true) |
416
| child_process-test.js:6:15:6:44 | url.par ... ).query |
@@ -179,6 +191,16 @@ nodes
179191
| third-party-command-injection.js:6:21:6:27 | command |
180192
| third-party-command-injection.js:6:21:6:27 | command |
181193
edges
194+
| actions.js:8:9:8:57 | title | actions.js:9:16:9:20 | title |
195+
| actions.js:8:17:8:57 | github. ... t.title | actions.js:8:9:8:57 | title |
196+
| actions.js:8:17:8:57 | github. ... t.title | actions.js:8:9:8:57 | title |
197+
| actions.js:9:16:9:20 | title | actions.js:9:8:9:22 | `echo ${title}` |
198+
| actions.js:9:16:9:20 | title | actions.js:9:8:9:22 | `echo ${title}` |
199+
| actions.js:18:9:18:63 | head_ref | actions.js:19:22:19:29 | head_ref |
200+
| actions.js:18:20:18:63 | github. ... ead.ref | actions.js:18:9:18:63 | head_ref |
201+
| actions.js:18:20:18:63 | github. ... ead.ref | actions.js:18:9:18:63 | head_ref |
202+
| actions.js:19:22:19:29 | head_ref | actions.js:19:14:19:31 | `echo ${head_ref}` |
203+
| actions.js:19:22:19:29 | head_ref | actions.js:19:14:19:31 | `echo ${head_ref}` |
182204
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:17:13:17:15 | cmd |
183205
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:17:13:17:15 | cmd |
184206
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:18:17:18:19 | cmd |
@@ -344,6 +366,8 @@ edges
344366
| third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command |
345367
| third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command |
346368
#select
369+
| actions.js:9:8:9:22 | `echo ${title}` | actions.js:8:17:8:57 | github. ... t.title | actions.js:9:8:9:22 | `echo ${title}` | This command line depends on a $@. | actions.js:8:17:8:57 | github. ... t.title | user-provided value |
370+
| actions.js:19:14:19:31 | `echo ${head_ref}` | actions.js:18:20:18:63 | github. ... ead.ref | actions.js:19:14:19:31 | `echo ${head_ref}` | This command line depends on a $@. | actions.js:18:20:18:63 | github. ... ead.ref | user-provided value |
347371
| 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 line depends on a $@. | child_process-test.js:6:25:6:31 | req.url | user-provided value |
348372
| 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 line depends on a $@. | child_process-test.js:6:25:6:31 | req.url | user-provided value |
349373
| child_process-test.js:19:17:19:19 | cmd | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:19:17:19:19 | cmd | This command line depends on a $@. | child_process-test.js:6:25:6:31 | req.url | user-provided value |

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
const github = require('@actions/github');
2+
const aexec = require('@actions/exec');
3+
const { exec } = require('child_process');
4+
5+
// function to echo title
6+
function echo_title() {
7+
// get the title from the event pull request
8+
const title = github.context.payload.pull_request.title;
9+
exec(`echo ${title}`, (err, stdout, stderr) => { // NOT OK
10+
if (err) {
11+
return;
12+
}
13+
});
14+
}
15+
16+
// function which passes the issue title into an exec
17+
function exec_head_ref() {
18+
const head_ref = github.context.payload.pull_request.head.ref;
19+
aexec.exec(`echo ${head_ref}`).then((res) => { // NOT OK
20+
console.log(res);
21+
});
22+
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
11
nodes
2+
| actions.js:4:6:4:16 | process.env |
3+
| actions.js:4:6:4:16 | process.env |
4+
| actions.js:4:6:4:29 | process ... _DATA'] |
5+
| actions.js:4:6:4:29 | process ... _DATA'] |
6+
| actions.js:7:15:7:15 | e |
7+
| actions.js:8:10:8:10 | e |
8+
| actions.js:8:10:8:23 | e['TEST_DATA'] |
9+
| actions.js:8:10:8:23 | e['TEST_DATA'] |
10+
| actions.js:12:6:12:16 | process.env |
11+
| actions.js:12:6:12:16 | process.env |
12+
| actions.js:14:6:14:21 | getInput('data') |
13+
| actions.js:14:6:14:21 | getInput('data') |
14+
| actions.js:14:6:14:21 | getInput('data') |
215
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
316
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
417
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
@@ -212,6 +225,16 @@ nodes
212225
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType |
213226
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType |
214227
edges
228+
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
229+
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
230+
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
231+
| actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] |
232+
| actions.js:7:15:7:15 | e | actions.js:8:10:8:10 | e |
233+
| actions.js:8:10:8:10 | e | actions.js:8:10:8:23 | e['TEST_DATA'] |
234+
| actions.js:8:10:8:10 | e | actions.js:8:10:8:23 | e['TEST_DATA'] |
235+
| actions.js:12:6:12:16 | process.env | actions.js:7:15:7:15 | e |
236+
| actions.js:12:6:12:16 | process.env | actions.js:7:15:7:15 | e |
237+
| actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') |
215238
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
216239
| command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
217240
| command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
@@ -400,6 +423,9 @@ edges
400423
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType | command-line-parameter-command-injection.js:146:10:146:38 | "cmd.sh ... zzaType |
401424
| command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType | command-line-parameter-command-injection.js:146:10:146:38 | "cmd.sh ... zzaType |
402425
#select
426+
| actions.js:4:6:4:29 | process ... _DATA'] | actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] | This command depends on an unsanitized $@. | actions.js:4:6:4:16 | process.env | environment variable |
427+
| actions.js:8:10:8:23 | e['TEST_DATA'] | actions.js:12:6:12:16 | process.env | actions.js:8:10:8:23 | e['TEST_DATA'] | This command depends on an unsanitized $@. | actions.js:12:6:12:16 | process.env | environment variable |
428+
| actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') | This command depends on an unsanitized $@. | actions.js:14:6:14:21 | getInput('data') | GitHub Actions user input |
403429
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line argument |
404430
| command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line argument |
405431
| command-line-parameter-command-injection.js:11:14:11:20 | args[0] | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line-parameter-command-injection.js:11:14:11:20 | args[0] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line argument |

‎javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { exec } from "@actions/exec";
2+
import { getInput } from "@actions/core";
3+
4+
exec(process.env['TEST_DATA']); // NOT OK
5+
exec(process.env['GITHUB_ACTION']); // OK
6+
7+
function test(e) {
8+
exec(e['TEST_DATA']); // NOT OK
9+
exec(e['GITHUB_ACTION']); // OK
10+
}
11+
12+
test(process.env);
13+
14+
exec(getInput('data')); // NOT OK

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ nodes
1313
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
1414
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
1515
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
16+
| actions.js:4:10:4:50 | github. ... message |
17+
| actions.js:4:10:4:50 | github. ... message |
18+
| actions.js:4:10:4:50 | github. ... message |
1619
| angularjs.js:10:22:10:36 | location.search |
1720
| angularjs.js:10:22:10:36 | location.search |
1821
| angularjs.js:10:22:10:36 | location.search |
@@ -191,6 +194,7 @@ edges
191194
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
192195
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
193196
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
197+
| actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message |
194198
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search |
195199
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search |
196200
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search |
@@ -306,6 +310,7 @@ edges
306310
| NoSQLCodeInjection.js:18:24:18:37 | req.body.query | NoSQLCodeInjection.js:18:24:18:31 | req.body | NoSQLCodeInjection.js:18:24:18:37 | req.body.query | This code execution depends on a $@. | NoSQLCodeInjection.js:18:24:18:31 | req.body | user-provided value |
307311
| NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name | NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name | This code execution depends on a $@. | NoSQLCodeInjection.js:19:36:19:43 | req.body | user-provided value |
308312
| NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name | NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name | This code execution depends on a $@. | NoSQLCodeInjection.js:22:36:22:43 | req.body | user-provided value |
313+
| actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message | This code execution depends on a $@. | actions.js:4:10:4:50 | github. ... message | user-provided value |
309314
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search | This code execution depends on a $@. | angularjs.js:10:22:10:36 | location.search | user-provided value |
310315
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search | This code execution depends on a $@. | angularjs.js:13:23:13:37 | location.search | user-provided value |
311316
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search | This code execution depends on a $@. | angularjs.js:16:28:16:42 | location.search | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ nodes
1313
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
1414
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
1515
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
16+
| actions.js:4:10:4:50 | github. ... message |
17+
| actions.js:4:10:4:50 | github. ... message |
18+
| actions.js:4:10:4:50 | github. ... message |
1619
| angularjs.js:10:22:10:36 | location.search |
1720
| angularjs.js:10:22:10:36 | location.search |
1821
| angularjs.js:10:22:10:36 | location.search |
@@ -195,6 +198,7 @@ edges
195198
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
196199
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
197200
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
201+
| actions.js:4:10:4:50 | github. ... message | actions.js:4:10:4:50 | github. ... message |
198202
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:36 | location.search |
199203
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:37 | location.search |
200204
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:42 | location.search |

0 commit comments

Comments
 (0)