Skip to content

Commit d1d92ae

Browse files
author
Alvaro Muñoz
committed
Create getATriggerEvent for Steps and refactor the code to use it
1 parent b2a3aaa commit d1d92ae

9 files changed

Lines changed: 25 additions & 360 deletions

File tree

ql/lib/codeql/actions/Ast.qll

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ class AstNode instanceof AstNodeImpl {
1717

1818
Job getEnclosingJob() { result = super.getEnclosingJob() }
1919

20+
Event getATriggerEvent() { result = super.getATriggerEvent() }
21+
2022
Workflow getEnclosingWorkflow() { result = super.getEnclosingWorkflow() }
2123

2224
CompositeAction getEnclosingCompositeAction() { result = super.getEnclosingCompositeAction() }
@@ -100,8 +102,6 @@ class Workflow extends AstNode instanceof WorkflowImpl {
100102

101103
Job getJob(string jobId) { result = super.getJob(jobId) }
102104

103-
Event getATriggerEvent() { result = super.getATriggerEvent() }
104-
105105
Permissions getPermissions() { result = super.getPermissions() }
106106

107107
Strategy getStrategy() { result = super.getStrategy() }
@@ -200,8 +200,6 @@ abstract class Job extends AstNode instanceof JobImpl {
200200

201201
Permissions getPermissions() { result = super.getPermissions() }
202202

203-
Event getATriggerEvent() { result = super.getATriggerEvent() }
204-
205203
Strategy getStrategy() { result = super.getStrategy() }
206204

207205
string getARunsOnLabel() { result = super.getARunsOnLabel() }

ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ abstract class AstNodeImpl extends TAstNode {
111111
result = this.getEnclosingCompositeAction().getACallerJob()
112112
}
113113

114+
/**
115+
* Gets and Event triggering this node.
116+
*/
117+
EventImpl getATriggerEvent() { result = this.getEnclosingJob().getATriggerEvent() }
118+
114119
/**
115120
* Gets the enclosing Step.
116121
*/
@@ -447,7 +452,7 @@ class CompositeActionImpl extends AstNodeImpl, TCompositeAction {
447452
)
448453
}
449454

450-
EventImpl getATriggerEvent() { result = this.getACallerJob().getATriggerEvent() }
455+
override EventImpl getATriggerEvent() { result = this.getACallerJob().getATriggerEvent() }
451456
}
452457

453458
class WorkflowImpl extends AstNodeImpl, TWorkflowNode {
@@ -486,7 +491,7 @@ class WorkflowImpl extends AstNodeImpl, TWorkflowNode {
486491
PermissionsImpl getPermissions() { result.getNode() = n.lookup("permissions") }
487492

488493
/** Gets the trigger event that starts this workflow. */
489-
EventImpl getATriggerEvent() { this.getOn().getAnEvent() = result }
494+
override EventImpl getATriggerEvent() { this.getOn().getAnEvent() = result }
490495

491496
/** Gets the strategy for this workflow. */
492497
StrategyImpl getStrategy() { result.getNode() = n.lookup("strategy") }
@@ -918,7 +923,7 @@ class JobImpl extends AstNodeImpl, TJobNode {
918923
StrategyImpl getStrategy() { result.getNode() = n.lookup("strategy") }
919924

920925
/** Gets the trigger event that starts this workflow. */
921-
EventImpl getATriggerEvent() {
926+
override EventImpl getATriggerEvent() {
922927
if this.getEnclosingWorkflow() instanceof ReusableWorkflowImpl
923928
then
924929
result = this.getEnclosingWorkflow().(ReusableWorkflowImpl).getACaller().getATriggerEvent()
@@ -1174,6 +1179,8 @@ class StepImpl extends AstNodeImpl, TStepNode {
11741179
result = super.getEnclosingJob()
11751180
}
11761181

1182+
override EventImpl getATriggerEvent() { result = this.getEnclosingJob().getATriggerEvent() }
1183+
11771184
EnvImpl getEnv() { result.getNode() = n.lookup("env") }
11781185

11791186
/** Gets the ID of this step, if any. */

ql/lib/codeql/actions/dataflow/FlowSources.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class GitCommandSource extends RemoteFlowSource, CommandSource {
9191
uses.getCallee() = "actions/checkout" and
9292
exists(uses.getArgument("ref")) and
9393
not uses.getArgument("ref").matches("%base%") and
94-
uses.getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers()
94+
uses.getATriggerEvent().getName() = checkoutTriggers()
9595
)
9696
or
9797
checkout instanceof GitMutableRefCheckout
@@ -239,7 +239,7 @@ private class CheckoutSource extends RemoteFlowSource, FileSource {
239239
uses.getCallee() = "actions/checkout" and
240240
exists(uses.getArgument("ref")) and
241241
not uses.getArgument("ref").matches("%base%") and
242-
uses.getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers()
242+
uses.getATriggerEvent().getName() = checkoutTriggers()
243243
)
244244
or
245245
this.asExpr() instanceof GitMutableRefCheckout

ql/lib/codeql/actions/security/ControlChecks.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ abstract class ControlCheck extends AstNode {
5757
// The check is effective against the event and category
5858
this.protectsCategoryAndEvent(category, event.getName()) and
5959
// The check can be triggered by the event
60-
this.getEnclosingJob().getATriggerEvent() = event
60+
this.getATriggerEvent() = event
6161
}
6262

6363
predicate dominates(AstNode node) {

ql/lib/codeql/actions/security/OutputClobberingQuery.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class OutputClobberingFromFileReadSink extends OutputClobberingSink {
3030
uses.getCallee() = "actions/checkout" and
3131
exists(uses.getArgument("ref")) and
3232
not uses.getArgument("ref").matches("%base%") and
33-
uses.getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers()
33+
uses.getATriggerEvent().getName() = checkoutTriggers()
3434
)
3535
or
3636
step instanceof GitMutableRefCheckout

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ string checkoutTriggers() {
1313
*/
1414
private module ActionsMutableRefCheckoutConfig implements DataFlow::ConfigSig {
1515
predicate isSource(DataFlow::Node source) {
16-
source.asExpr().getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers() and
16+
source.asExpr().getATriggerEvent().getName() = checkoutTriggers() and
1717
(
1818
// remote flow sources
1919
source instanceof ArtifactSource
@@ -79,7 +79,7 @@ module ActionsMutableRefCheckoutFlow = TaintTracking::Global<ActionsMutableRefCh
7979

8080
private module ActionsSHACheckoutConfig implements DataFlow::ConfigSig {
8181
predicate isSource(DataFlow::Node source) {
82-
source.asExpr().getEnclosingJob().getATriggerEvent().getName() =
82+
source.asExpr().getATriggerEvent().getName() =
8383
["pull_request_target", "workflow_run", "workflow_call", "issue_comment"] and
8484
(
8585
// `ref` argument contains the PR head/merge commit sha
@@ -283,7 +283,7 @@ class ActionsSHACheckout extends SHACheckoutStep instanceof UsesStep {
283283
class GitMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
284284
GitMutableRefCheckout() {
285285
exists(string cmd | this.getScript().getACommand() = cmd |
286-
this.getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers() and
286+
this.getATriggerEvent().getName() = checkoutTriggers() and
287287
cmd.regexpMatch("git\\s+(fetch|pull).*") and
288288
(
289289
(containsHeadRef(cmd) or containsPullRequestNumber(cmd))
@@ -307,7 +307,7 @@ class GitMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
307307
class GitSHACheckout extends SHACheckoutStep instanceof Run {
308308
GitSHACheckout() {
309309
exists(string cmd | this.getScript().getACommand() = cmd |
310-
this.getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers() and
310+
this.getATriggerEvent().getName() = checkoutTriggers() and
311311
cmd.regexpMatch("git\\s+(fetch|pull).*") and
312312
(
313313
containsHeadSHA(cmd)
@@ -328,7 +328,7 @@ class GitSHACheckout extends SHACheckoutStep instanceof Run {
328328
class GhMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
329329
GhMutableRefCheckout() {
330330
exists(string cmd | this.getScript().getACommand() = cmd |
331-
this.getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers() and
331+
this.getATriggerEvent().getName() = checkoutTriggers() and
332332
cmd.regexpMatch(".*(gh|hub)\\s+pr\\s+checkout.*") and
333333
(
334334
(containsHeadRef(cmd) or containsPullRequestNumber(cmd))
@@ -351,7 +351,7 @@ class GhMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
351351
class GhSHACheckout extends SHACheckoutStep instanceof Run {
352352
GhSHACheckout() {
353353
exists(string cmd | this.getScript().getACommand() = cmd |
354-
this.getEnclosingJob().getATriggerEvent().getName() = checkoutTriggers() and
354+
this.getATriggerEvent().getName() = checkoutTriggers() and
355355
cmd.regexpMatch("gh\\s+pr\\s+checkout.*") and
356356
(
357357
containsHeadSHA(cmd)

ql/src/Security/CWE-829/ArtifactPoisoningPathTraversal.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import codeql.actions.security.UseOfKnownVulnerableActionQuery
1818

1919
from UsesStep download, KnownVulnerableAction vulnerable_action, Event event
2020
where
21-
event = download.getEnclosingJob().getATriggerEvent() and
21+
event = download.getATriggerEvent() and
2222
vulnerable_action.getVulnerableAction() = download.getCallee() and
2323
download.getCallee() = "actions/download-artifact" and
2424
(

0 commit comments

Comments
 (0)