Skip to content

Commit 6bab41c

Browse files
erik-krogherik-krogh
authored
Merge pull request github#5350 from JarLob/actions
github actions queries
2 parents c0e1df4 + a9ed331 commit 6bab41c

26 files changed

+952
-0
lines changed

‎javascript/ql/src/experimental/Security/CWE-094/ExpressionInjection.qhelp

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
8+
<p>
9+
10+
Using user-controlled input in GitHub Actions may lead to
11+
code injection in contexts like <i>run:</i> or <i>script:</i>.
12+
13+
</p>
14+
15+
</overview>
16+
17+
<recommendation>
18+
19+
<p>
20+
21+
The best practice to avoid code injection vulnerabilities
22+
in GitHub workflows is to set the untrusted input value of the expression
23+
to an intermediate environment variable.
24+
25+
</p>
26+
27+
</recommendation>
28+
29+
<example>
30+
31+
<p>
32+
33+
The following example lets a user inject an arbitrary shell command:
34+
35+
</p>
36+
37+
<sample src="examples/comment_issue_bad.yml" />
38+
39+
<p>
40+
41+
The following example uses shell syntax to read
42+
the environment variable and will prevent the attack:
43+
44+
</p>
45+
46+
<sample src="examples/comment_issue_good.yml" />
47+
48+
</example>
49+
50+
<references>
51+
<li>GitHub Security Lab Research: <a href="https://securitylab.github.com/research/github-actions-untrusted-input">Keeping your GitHub Actions and workflows secure: Untrusted input</a>.</li>
52+
</references>
53+
54+
</qhelp>

‎javascript/ql/src/experimental/Security/CWE-094/ExpressionInjection.ql

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/**
2+
* @name Expression injection in Actions
3+
* @description Using user-controlled GitHub Actions contexts like `run:` or `script:` may allow a malicious
4+
* user to inject code into the GitHub action.
5+
* @kind problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id js/actions/injection
9+
* @tags actions
10+
* security
11+
* external/cwe/cwe-094
12+
*/
13+
14+
import javascript
15+
import experimental.semmle.javascript.Actions
16+
17+
bindingset[context]
18+
private predicate isExternalUserControlledIssue(string context) {
19+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*issue\\s*\\.\\s*title\\b") or
20+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*issue\\s*\\.\\s*body\\b")
21+
}
22+
23+
bindingset[context]
24+
private predicate isExternalUserControlledPullRequest(string context) {
25+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*title\\b") or
26+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*body\\b") or
27+
context
28+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*label\\b") or
29+
context
30+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*repo\\s*\\.\\s*default_branch\\b") or
31+
context
32+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*ref\\b")
33+
}
34+
35+
bindingset[context]
36+
private predicate isExternalUserControlledReview(string context) {
37+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*review\\s*\\.\\s*body\\b") or
38+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*review_comment\\s*\\.\\s*body\\b")
39+
}
40+
41+
bindingset[context]
42+
private predicate isExternalUserControlledComment(string context) {
43+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*comment\\s*\\.\\s*body\\b")
44+
}
45+
46+
bindingset[context]
47+
private predicate isExternalUserControlledGollum(string context) {
48+
context
49+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pages(?:\\[[0-9]\\]|\\s*\\.\\s*\\*)+\\s*\\.\\s*page_name\\b")
50+
}
51+
52+
bindingset[context]
53+
private predicate isExternalUserControlledCommit(string context) {
54+
context
55+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits(?:\\[[0-9]\\]|\\s*\\.\\s*\\*)+\\s*\\.\\s*message\\b") or
56+
context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*message\\b") or
57+
context
58+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*author\\s*\\.\\s*email\\b") or
59+
context
60+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*author\\s*\\.\\s*name\\b") or
61+
context
62+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits(?:\\[[0-9]\\]|\\s*\\.\\s*\\*)+\\s*\\.\\s*author\\s*\\.\\s*email\\b") or
63+
context
64+
.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits(?:\\[[0-9]\\]|\\s*\\.\\s*\\*)+\\s*\\.\\s*author\\s*\\.\\s*name\\b") or
65+
context.regexpMatch("\\bgithub\\s*\\.\\s*head_ref\\b")
66+
}
67+
68+
from Actions::Run run, string context, Actions::On on
69+
where
70+
run.getAReferencedExpression() = context and
71+
run.getStep().getJob().getWorkflow().getOn() = on and
72+
(
73+
exists(on.getNode("issues")) and
74+
isExternalUserControlledIssue(context)
75+
or
76+
exists(on.getNode("pull_request_target")) and
77+
isExternalUserControlledPullRequest(context)
78+
or
79+
(exists(on.getNode("pull_request_review_comment")) or exists(on.getNode("pull_request_review"))) and
80+
isExternalUserControlledReview(context)
81+
or
82+
(exists(on.getNode("issue_comment")) or exists(on.getNode("pull_request_target"))) and
83+
isExternalUserControlledComment(context)
84+
or
85+
exists(on.getNode("gollum")) and
86+
isExternalUserControlledGollum(context)
87+
or
88+
exists(on.getNode("pull_request_target")) and
89+
isExternalUserControlledCommit(context)
90+
)
91+
select run,
92+
"Potential injection from the " + context +
93+
" context, which may be controlled by an external user."

‎javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.qhelp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
8+
<p>
9+
10+
Combining <i>pull_request_target</i> workflow trigger with an explicit checkout
11+
of an untrusted pull request is a dangerous practice
12+
that may lead to repository compromise.
13+
14+
</p>
15+
16+
</overview>
17+
18+
<recommendation>
19+
20+
<p>
21+
22+
The best practice is to handle the potentially untrusted pull request
23+
via the <i>pull_request</i> trigger so that it is isolated in
24+
an unprivileged environment. The workflow processing the pull request
25+
should then store any results like code coverage or failed/passed tests
26+
in artifacts and exit. The following workflow then starts on <i>workflow_run</i>
27+
where it is granted write permission to the target repository and access to
28+
repository secrets, so that it can download the artifacts and make
29+
any necessary modifications to the repository or interact with third party services
30+
that require repository secrets (e.g. API tokens).
31+
32+
</p>
33+
34+
</recommendation>
35+
36+
<example>
37+
38+
<p>
39+
40+
The following example allows unauthorized repository modification
41+
and secrets exfiltration:
42+
43+
</p>
44+
45+
<sample src="examples/pull_request_target_bad.yml" />
46+
47+
<p>
48+
49+
The following example uses two workflows to handle potentially untrusted
50+
pull request in a secure manner. The receive_pr.yml is triggered first:
51+
52+
</p>
53+
54+
<sample src="examples/receive_pr.yml" />
55+
<p>The comment_pr.yml is triggered after receive_pr.yml completes:</p>
56+
<sample src="examples/comment_pr.yml" />
57+
58+
</example>
59+
60+
<references>
61+
<li>GitHub Security Lab Research: <a href="https://securitylab.github.com/research/github-actions-preventing-pwn-requests">Keeping your GitHub Actions and workflows secure: Preventing pwn requests</a>.</li>
62+
</references>
63+
64+
</qhelp>

‎javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.ql

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/**
2+
* @name Checkout of untrusted code in trusted context
3+
* @description Workflows triggered on `pull_request_target` have read/write access to the base repository and access to secrets.
4+
* By explicitly checking out and running the build script from a fork the untrusted code is running in an environment
5+
* that is able to push to the base repository and to access secrets.
6+
* @kind problem
7+
* @problem.severity warning
8+
* @precision low
9+
* @id js/actions/pull_request_target
10+
* @tags actions
11+
* security
12+
* external/cwe/cwe-094
13+
*/
14+
15+
import javascript
16+
import experimental.semmle.javascript.Actions
17+
18+
/**
19+
* Action step that doesn't contain `actor` or `label` check in `if:` or
20+
* the check requires manual analysis.
21+
*/
22+
class ProbableStep extends Actions::Step {
23+
// some simplistic checks to eleminate likely false positives:
24+
ProbableStep() {
25+
// no if at all
26+
not exists(this.getIf().getValue())
27+
or
28+
// needs manual analysis if there is OR
29+
this.getIf().getValue().matches("%||%")
30+
or
31+
// labels can be assigned by owners only
32+
not exists(
33+
this.getIf()
34+
.getValue()
35+
.regexpFind("\\bcontains\\s*\\(\\s*github\\s*\\.\\s*event\\s*\\.\\s*(?:issue|pull_request)\\s*\\.\\s*labels\\b",
36+
_, _)
37+
) and
38+
not exists(
39+
this.getIf()
40+
.getValue()
41+
.regexpFind("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*label\\s*\\.\\s*name\\s*==", _, _)
42+
) and
43+
// actor check means only the user is able to run it
44+
not exists(this.getIf().getValue().regexpFind("\\bgithub\\s*\\.\\s*actor\\s*==", _, _))
45+
}
46+
}
47+
48+
/**
49+
* Action job that doesn't contain `actor` or `label` check in `if:` or
50+
* the check requires manual analysis.
51+
*/
52+
class ProbableJob extends Actions::Job {
53+
// some simplistic checks to eleminate likely false positives:
54+
ProbableJob() {
55+
// no if at all
56+
not exists(this.getIf().getValue())
57+
or
58+
// needs manual analysis if there is OR
59+
this.getIf().getValue().matches("%||%")
60+
or
61+
// labels can be assigned by owners only
62+
not exists(
63+
this.getIf()
64+
.getValue()
65+
.regexpFind("\\bcontains\\s*\\(\\s*github\\s*\\.\\s*event\\s*\\.\\s*(?:issue|pull_request)\\s*\\.\\s*labels\\b",
66+
_, _)
67+
) and
68+
not exists(
69+
this.getIf()
70+
.getValue()
71+
.regexpFind("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*label\\s*\\.\\s*name\\s*==", _, _)
72+
) and
73+
// actor check means only the user is able to run it
74+
not exists(this.getIf().getValue().regexpFind("\\bgithub\\s*\\.\\s*actor\\s*==", _, _))
75+
}
76+
}
77+
78+
/**
79+
* Action step that doesn't contain `actor` or `label` check in `if:` or
80+
*/
81+
class ProbablePullRequestTarget extends Actions::On, Actions::MappingOrSequenceOrScalar {
82+
ProbablePullRequestTarget() {
83+
exists(YAMLNode prtNode |
84+
// The `on:` is triggered on `pull_request_target`
85+
this.getNode("pull_request_target") = prtNode and
86+
(
87+
// and either doesn't contain `types` filter
88+
not exists(prtNode.getAChild())
89+
or
90+
// or has the filter, that is something else than just [labeled]
91+
exists(Actions::MappingOrSequenceOrScalar prt, Actions::MappingOrSequenceOrScalar types |
92+
types = prt.getNode("types") and
93+
prtNode = prt and
94+
(
95+
not types.getElementCount() = 1 or
96+
not exists(types.getNode("labeled"))
97+
)
98+
)
99+
)
100+
)
101+
}
102+
}
103+
104+
from
105+
Actions::Ref ref, Actions::Uses uses, Actions::Step step, Actions::Job job,
106+
ProbablePullRequestTarget pullRequestTarget
107+
where
108+
pullRequestTarget.getWorkflow() = job.getWorkflow() and
109+
uses.getStep() = step and
110+
ref.getWith().getStep() = step and
111+
step.getJob() = job and
112+
uses.getGitHubRepository() = "actions/checkout" and
113+
(
114+
ref.getValue().matches("%github.event.pull_request.head.ref%") or
115+
ref.getValue().matches("%github.event.pull_request.head.sha%") or
116+
ref.getValue().matches("%github.event.pull_request.number%") or
117+
ref.getValue().matches("%github.event.number%") or
118+
ref.getValue().matches("%github.head_ref%")
119+
) and
120+
step instanceof ProbableStep and
121+
job instanceof ProbableJob
122+
select step, "Potential unsafe checkout of untrusted pull request on `pull_request_target`"

‎javascript/ql/src/experimental/Security/CWE-094/examples/comment_issue_bad.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
on: issue_comment
2+
3+
jobs:
4+
echo-body:
5+
runs-on: ubuntu-latest
6+
steps:
7+
- run: |
8+
echo '${{ github.event.comment.body }}'

‎javascript/ql/src/experimental/Security/CWE-094/examples/comment_issue_good.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
on: issue_comment
2+
3+
jobs:
4+
echo-body:
5+
runs-on: ubuntu-latest
6+
steps:
7+
- env:
8+
BODY: ${{ github.event.issue.body }}
9+
run: |
10+
echo '$BODY'

0 commit comments

Comments
 (0)