Skip to content

Commit 147da50

Browse files
author
Alvaro Muñoz
committed
Use Taint Tracking to track PR refs to checkout's ref argument
1 parent bd0c762 commit 147da50

File tree

1 file changed

+95
-55
lines changed

1 file changed

+95
-55
lines changed

‎ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll

Lines changed: 95 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,90 @@
11
import actions
2-
import codeql.actions.DataFlow
2+
private import codeql.actions.DataFlow
3+
private import codeql.actions.TaintTracking
34

4-
string getStepCWD() {
5-
// TODO: This should be the path of the git command.
6-
// Read if from the step's CWD, workspace or look for a cd command.
7-
result = "?"
5+
/**
6+
* A taint-tracking configuration for PR HEAD references flowing
7+
* into actions/checkout's ref argument.
8+
*/
9+
private module ActionsMutableRefCheckoutConfig implements DataFlow::ConfigSig {
10+
predicate isSource(DataFlow::Node source) {
11+
// `ref` argument contains the PR id/number or head ref
12+
exists(Expression e |
13+
source.asExpr() = e and
14+
(
15+
containsHeadRef(e.getExpression()) or
16+
containsPullRequestNumber(e.getExpression())
17+
)
18+
)
19+
or
20+
// 3rd party actions returning the PR head ref
21+
exists(StepsExpression e, UsesStep step |
22+
source.asExpr() = e and
23+
e.getStepId() = step.getId() and
24+
(
25+
step.getCallee() = "eficode/resolve-pr-refs" and e.getFieldName() = "head_ref"
26+
or
27+
step.getCallee() = "xt0rted/pull-request-comment-branch" and e.getFieldName() = "head_ref"
28+
or
29+
step.getCallee() = "alessbell/pull-request-comment-branch" and e.getFieldName() = "head_ref"
30+
or
31+
step.getCallee() = "gotson/pull-request-comment-branch" and e.getFieldName() = "head_ref"
32+
or
33+
step.getCallee() = "potiuk/get-workflow-origin" and
34+
e.getFieldName() = ["sourceHeadBranch", "pullRequestNumber"]
35+
or
36+
step.getCallee() = "github/branch-deploy" and e.getFieldName() = ["ref", "fork_ref"]
37+
)
38+
)
39+
}
40+
41+
predicate isSink(DataFlow::Node sink) {
42+
exists(Uses uses |
43+
uses.getCallee() = "actions/checkout" and
44+
uses.getArgumentExpr("ref") = sink.asExpr()
45+
)
46+
}
47+
}
48+
49+
module ActionsMutableRefCheckoutFlow = TaintTracking::Global<ActionsMutableRefCheckoutConfig>;
50+
51+
private module ActionsSHACheckoutConfig implements DataFlow::ConfigSig {
52+
predicate isSource(DataFlow::Node source) {
53+
// `ref` argument contains the PR head/merge commit sha
54+
exists(Expression e |
55+
source.asExpr() = e and
56+
containsHeadSHA(e.getExpression())
57+
)
58+
or
59+
// 3rd party actions returning the PR head sha
60+
exists(StepsExpression e, UsesStep step |
61+
source.asExpr() = e and
62+
e.getStepId() = step.getId() and
63+
(
64+
step.getCallee() = "eficode/resolve-pr-refs" and e.getFieldName() = "head_sha"
65+
or
66+
step.getCallee() = "xt0rted/pull-request-comment-branch" and e.getFieldName() = "head_sha"
67+
or
68+
step.getCallee() = "alessbell/pull-request-comment-branch" and e.getFieldName() = "head_sha"
69+
or
70+
step.getCallee() = "gotson/pull-request-comment-branch" and e.getFieldName() = "head_sha"
71+
or
72+
step.getCallee() = "potiuk/get-workflow-origin" and
73+
e.getFieldName() = ["sourceHeadSha", "mergeCommitSha"]
74+
)
75+
)
76+
}
77+
78+
predicate isSink(DataFlow::Node sink) {
79+
exists(Uses uses |
80+
uses.getCallee() = "actions/checkout" and
81+
uses.getArgumentExpr("ref") = sink.asExpr()
82+
)
83+
}
884
}
985

86+
module ActionsSHACheckoutFlow = TaintTracking::Global<ActionsSHACheckoutConfig>;
87+
1088
bindingset[s]
1189
predicate containsPullRequestNumber(string s) {
1290
exists(
@@ -73,6 +151,12 @@ predicate containsHeadRef(string s) {
73151
)
74152
}
75153

154+
private string getStepCWD() {
155+
// TODO: This should be the path of the git command.
156+
// Read if from the step's CWD, workspace or look for a cd command.
157+
result = "?"
158+
}
159+
76160
/** Checkout of a Pull Request HEAD */
77161
abstract class PRHeadCheckoutStep extends Step {
78162
abstract string getPath();
@@ -89,35 +173,9 @@ class ActionsMutableRefCheckout extends MutableRefCheckoutStep instanceof UsesSt
89173
ActionsMutableRefCheckout() {
90174
this.getCallee() = "actions/checkout" and
91175
(
92-
// ref argument contains the PR id/number or head ref/sha
93-
exists(Expression e |
94-
(
95-
containsHeadRef(e.getExpression()) or
96-
containsPullRequestNumber(e.getExpression())
97-
) and
98-
DataFlow::hasLocalFlowExpr(e, this.getArgumentExpr("ref"))
99-
)
100-
or
101-
// 3rd party actions returning the PR head sha/ref
102-
exists(UsesStep step |
103-
(
104-
step.getCallee() =
105-
[
106-
"eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch",
107-
"alessbell/pull-request-comment-branch", "gotson/pull-request-comment-branch"
108-
] and
109-
// TODO: This should be read step of the head_sha or head_ref output vars
110-
this.getArgument("ref").regexpMatch(".*(head_ref).*")
111-
or
112-
step.getCallee() = "potiuk/get-workflow-origin" and
113-
// TODO: This should be read step of the ref output var
114-
this.getArgument("ref").matches("%." + ["sourceHeadBranch", "pullRequestNumber"])
115-
or
116-
step.getCallee() = "github/branch-deploy" and
117-
// TODO: This should be read step of the ref output var
118-
this.getArgument("ref").matches("%.ref%")
119-
) and
120-
DataFlow::hasLocalFlowExpr(step, this.getArgumentExpr("ref"))
176+
exists(ActionsMutableRefCheckoutFlow::PathNode sink |
177+
ActionsMutableRefCheckoutFlow::flowPath(_, sink) and
178+
sink.getNode().asExpr() = this.getArgumentExpr("ref")
121179
)
122180
or
123181
// heuristic base on the step id and field name
@@ -159,27 +217,9 @@ class ActionsSHACheckout extends SHACheckoutStep instanceof UsesStep {
159217
ActionsSHACheckout() {
160218
this.getCallee() = "actions/checkout" and
161219
(
162-
// ref argument contains the PR id/number or head ref/sha
163-
exists(Expression e |
164-
containsHeadSHA(e.getExpression()) and
165-
DataFlow::hasLocalFlowExpr(e, this.getArgumentExpr("ref"))
166-
)
167-
or
168-
// 3rd party actions returning the PR head sha/ref
169-
exists(UsesStep step |
170-
(
171-
step.getCallee() =
172-
[
173-
"eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch",
174-
"alessbell/pull-request-comment-branch", "gotson/pull-request-comment-branch"
175-
] and
176-
this.getArgument("ref").regexpMatch(".*(head_sha).*")
177-
or
178-
step.getCallee() = "potiuk/get-workflow-origin" and
179-
// TODO: This should be read step of the ref output var
180-
this.getArgument("ref").matches("%." + ["sourceHeadSha", "mergeCommitSha"])
181-
) and
182-
DataFlow::hasLocalFlowExpr(step, this.getArgumentExpr("ref"))
220+
exists(ActionsSHACheckoutFlow::PathNode sink |
221+
ActionsSHACheckoutFlow::flowPath(_, sink) and
222+
sink.getNode().asExpr() = this.getArgumentExpr("ref")
183223
)
184224
or
185225
// heuristic base on the step id and field name

0 commit comments

Comments
 (0)