Skip to content

Commit ae4ce2e

Browse files
committed
Merge github/main into feature/actions-external-workflow-models
Preserve both Actions test data extensions and regenerate the conflicting CWE-094 and CWE-829 expected results.
2 parents 52ee51b + 1c0ae0f commit ae4ce2e

69 files changed

Lines changed: 4165 additions & 1653 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

MODULE.bazel

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,10 +326,11 @@ ripunzip_archive = use_repo_rule("//misc/ripunzip:ripunzip.bzl", "ripunzip_archi
326326
# go to https://github.com/GoogleChrome/ripunzip/releases to find latest version and corresponding sha256s
327327
ripunzip_archive(
328328
name = "ripunzip",
329-
sha256_linux = "71482d7a7e4ea9176d5596161c49250c34b136b157c45f632b1111323fbfc0de",
330-
sha256_macos_arm = "604194ab13f0aba3972995d995f11002b8fc285c8170401fcd46655065df20c9",
331-
sha256_macos_intel = "65367b94fd579d93d46f2d2595cc4c9a60cfcf497e3c824f9d1a7b80fa8bd38a",
332-
sha256_windows = "ac3874075def2b9e5074a3b5945005ab082cc6e689e1de658da8965bc23e643e",
329+
sha256_linux_arm64 = "a282740ef376ff8dc0de3c589b7457598db15cc045b7daa688f15531612e0bbe",
330+
sha256_linux_x64 = "71482d7a7e4ea9176d5596161c49250c34b136b157c45f632b1111323fbfc0de",
331+
sha256_macos_arm64 = "604194ab13f0aba3972995d995f11002b8fc285c8170401fcd46655065df20c9",
332+
sha256_macos_x64 = "65367b94fd579d93d46f2d2595cc4c9a60cfcf497e3c824f9d1a7b80fa8bd38a",
333+
sha256_windows_x64 = "ac3874075def2b9e5074a3b5945005ab082cc6e689e1de658da8965bc23e643e",
333334
version = "2.0.4",
334335
)
335336

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* GitHub Actions analysis now recognizes untrusted data in `github.event.merge_group` for workflows triggered by the `merge_group` event.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ Event getRelevantNonArtifactEventInPrivilegedContext(DataFlow::Node sink) {
151151
private module EnvVarInjectionConfig implements DataFlow::ConfigSig {
152152
predicate isSource(DataFlow::Node source) {
153153
source instanceof RemoteFlowSource and
154-
not source.(RemoteFlowSource).getSourceType() = ["branch", "username"]
154+
not source.(RemoteFlowSource).getSourceType() = ["branch", "label", "username"]
155155
}
156156

157157
predicate isSink(DataFlow::Node sink) { sink instanceof EnvVarInjectionSink }

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

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,43 @@ class WorkflowCommandClobberingFromEnvVarSink extends OutputClobberingSink {
111111
}
112112
}
113113

114+
private string jqSafeOptionRegexp() {
115+
result = "-[acCMeRnSs]+"
116+
or
117+
result =
118+
"--(ascii-output|color-output|compact-output|exit-status|monochrome-output|null-input|" +
119+
"raw-input|slurp|sort-keys|unbuffered)"
120+
}
121+
122+
private string jqSimpleFilterRegexp() {
123+
result = "\\."
124+
or
125+
result = "\\.[A-Za-z_][A-Za-z0-9_]*(\\.[A-Za-z_][A-Za-z0-9_]*|\\[[0-9]+\\])*"
126+
}
127+
128+
private string jqSimpleFilterArgumentRegexp() {
129+
result = jqSimpleFilterRegexp()
130+
or
131+
result = "'" + jqSimpleFilterRegexp() + "'"
132+
or
133+
result = "\"" + jqSimpleFilterRegexp() + "\""
134+
}
135+
136+
private string jqLiteralInputRegexp() {
137+
result = "[A-Za-z0-9_./][A-Za-z0-9_./-]*"
138+
or
139+
result = "\\$GITHUB_EVENT_PATH"
140+
or
141+
result = "\\$\\{GITHUB_EVENT_PATH\\}"
142+
}
143+
144+
bindingset[command]
145+
private predicate jqProducesJsonEncodedOutput(string command) {
146+
command
147+
.regexpMatch("jq(\\s+" + jqSafeOptionRegexp() + ")*\\s+" + jqSimpleFilterArgumentRegexp() +
148+
"(\\s+" + jqSafeOptionRegexp() + ")*(\\s+" + jqLiteralInputRegexp() + ")*")
149+
}
150+
114151
/**
115152
* - id: clob1
116153
* run: |
@@ -159,13 +196,17 @@ class WorkflowCommandClobberingFromFileReadSink extends OutputClobberingSink {
159196
clobbering_cmd.regexpMatch(["ls", Bash::fileReadCommand()] + "\\s.*") and
160197
(
161198
// - run: echo "foo=$(<pr-id.txt)"
162-
clobbering_stmt.regexpMatch("echo.*" + clobbering_cmd + ".*")
199+
exists(string echo, int echoOffset |
200+
echo = clobbering_stmt.regexpFind("\\becho\\s+", _, echoOffset) and
201+
clobbering_stmt.indexOf(clobbering_cmd, 0, echoOffset + echo.length()) >= 0
202+
)
163203
or
164204
// A file content is printed to stdout
165205
// - run: cat pr-id.txt
166206
clobbering_stmt.indexOf(clobbering_cmd) = 0
167207
)
168-
)
208+
) and
209+
not jqProducesJsonEncodedOutput(clobbering_cmd)
169210
)
170211
}
171212
}

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,3 +382,50 @@ class GhSHACheckout extends SHACheckoutStep instanceof Run {
382382

383383
override string getPath() { result = this.(Run).getWorkingDirectory() }
384384
}
385+
386+
private predicate isRunCheckoutReference(
387+
PRHeadCheckoutStep checkout, Expression reference, string variable
388+
) {
389+
reference = checkout.(Run).getInScopeEnvVarExpr(variable) and
390+
(
391+
checkout instanceof SHACheckoutStep and containsHeadSHA(reference.getExpression())
392+
or
393+
checkout instanceof MutableRefCheckoutStep and
394+
(
395+
containsHeadRef(reference.getExpression()) or
396+
containsPullRequestNumber(reference.getExpression())
397+
)
398+
) and
399+
exists(string command |
400+
checkout.(Run).getScript().getACommand() = command and
401+
exists(command.regexpFind(variable, _, _))
402+
)
403+
}
404+
405+
/** Gets the expression that controls the untrusted checkout, if one can be identified. */
406+
AstNode getCheckoutReference(PRHeadCheckoutStep checkout) {
407+
exists(UsesStep uses | uses = checkout |
408+
result = uses.getArgumentExpr("ref")
409+
or
410+
not exists(uses.getArgumentExpr("ref")) and result = uses.getArgumentExpr("repository")
411+
)
412+
or
413+
isRunCheckoutReference(checkout, result, _)
414+
or
415+
checkout instanceof Run and
416+
result = checkout and
417+
not isRunCheckoutReference(checkout, _, _)
418+
}
419+
420+
/** Gets a display label for the expression that controls the untrusted checkout. */
421+
string getCheckoutReferenceText(AstNode reference) {
422+
result = reference.(Expression).toString()
423+
or
424+
not reference instanceof Expression and result = "the checkout command"
425+
}
426+
427+
/** Adds checkout-reference provenance before the checkout step in path queries. */
428+
predicate checkoutReferenceEdge(AstNode predecessor, AstNode successor) {
429+
predecessor = getCheckoutReference(successor) and
430+
not predecessor = successor
431+
}

actions/ql/lib/ext/config/context_event_map.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ extensions:
1919
- ["gollum", "github.event.changes"]
2020
- ["pull_request_comment", "github.event.comment"]
2121
- ["pull_request_comment", "github.event.pull_request"]
22+
- ["merge_group", "github.event.merge_group"]
2223
- ["pull_request_comment", "github.head_ref"]
2324
- ["pull_request_comment", "github.event.changes"]
2425
- ["pull_request_review", "github.event.pull_request"]

actions/ql/lib/ext/config/externally_triggereable_events.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@ extensions:
1717
- ["workflow_run"] # depending on branch filter
1818
- ["workflow_call"] # depending on caller
1919
- ["workflow_dispatch"]
20-
- ["scheduled"]
20+
- ["schedule"]

actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,16 @@ import codeql.actions.dataflow.FlowSources
1919
import EnvVarInjectionFlow::PathGraph
2020
import codeql.actions.security.ControlChecks
2121

22+
bindingset[source, event]
23+
pragma[inline_late]
24+
private predicate hasSameEventName(RemoteFlowSource source, Event event) {
25+
source.getEventName() = event.getName()
26+
}
27+
2228
from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink, Event event
2329
where
2430
EnvVarInjectionFlow::flowPath(source, sink) and
31+
hasSameEventName(source.getNode(), event) and
2532
// exclude paths to file read sinks from non-artifact sources
2633
(
2734
// source is text

actions/ql/src/Security/CWE-349/CachePoisoningViaPoisonableStep.ql

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,30 @@ import codeql.actions.security.CachePoisoningQuery
1818
import codeql.actions.security.PoisonableSteps
1919
import codeql.actions.security.ControlChecks
2020

21-
query predicate edges(Step a, Step b) { a.getNextStep() = b }
21+
query predicate edges(AstNode predecessor, AstNode successor) {
22+
predecessor.(Step).getNextStep() = successor
23+
or
24+
checkoutReferenceEdge(predecessor, successor)
25+
}
2226

23-
from LocalJob job, Event event, Step source, Step step, string message, string path
27+
from
28+
LocalJob job, Event event, Step source, Step step, string message, string path,
29+
AstNode untrustedInput, string untrustedInputText
2430
where
2531
// the job checkouts untrusted code from a pull request or downloads an untrusted artifact
2632
job.getAStep() = source and
2733
(
2834
source instanceof PRHeadCheckoutStep and
29-
message = "due to privilege checkout of untrusted code." and
30-
path = source.(PRHeadCheckoutStep).getPath()
35+
message = "due to privilege checkout of untrusted code from" and
36+
path = source.(PRHeadCheckoutStep).getPath() and
37+
untrustedInput = getCheckoutReference(source) and
38+
untrustedInputText = getCheckoutReferenceText(untrustedInput)
3139
or
3240
source instanceof UntrustedArtifactDownloadStep and
33-
message = "due to downloading an untrusted artifact." and
34-
path = source.(UntrustedArtifactDownloadStep).getPath()
41+
message = "due to downloading" and
42+
path = source.(UntrustedArtifactDownloadStep).getPath() and
43+
untrustedInput = source and
44+
untrustedInputText = "an untrusted artifact"
3545
) and
3646
// the checkout/download is not controlled by an access check
3747
not exists(ControlCheck check |
@@ -57,6 +67,6 @@ where
5767
step instanceof PoisonableStep and
5868
// excluding privileged workflows since they can be exploited in easier circumstances
5969
not job.isPrivileged()
60-
select step, source, step,
61-
"Potential cache poisoning in the context of the default branch " + message + " ($@).", event,
62-
event.getName()
70+
select step, untrustedInput, step,
71+
"Potential cache poisoning in the context of the default branch " + message + " $@. ($@).",
72+
untrustedInput, untrustedInputText, event, event.getName()

actions/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,18 @@ import codeql.actions.security.UntrustedCheckoutQuery
1818
import codeql.actions.security.PoisonableSteps
1919
import codeql.actions.security.ControlChecks
2020

21-
query predicate edges(Step a, Step b) { a.getNextStep() = b }
21+
query predicate edges(AstNode predecessor, AstNode successor) {
22+
predecessor.(Step).getNextStep() = successor
23+
or
24+
checkoutReferenceEdge(predecessor, successor)
25+
}
2226

23-
from PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event
27+
from
28+
PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event, AstNode checkoutReference,
29+
string checkoutReferenceText
2430
where
31+
checkoutReference = getCheckoutReference(checkout) and
32+
checkoutReferenceText = getCheckoutReferenceText(checkoutReference) and
2533
// the checkout is followed by a known poisonable step
2634
checkout.getAFollowingStep() = poisonable and
2735
(
@@ -51,6 +59,6 @@ where
5159
event.getName() = checkoutTriggers() and
5260
not exists(ControlCheck check | check.protects(checkout, event, "untrusted-checkout")) and
5361
not exists(ControlCheck check | check.protects(poisonable, event, "untrusted-checkout"))
54-
select checkout, checkout, poisonable,
55-
"Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@).",
56-
event, event.getName()
62+
select checkout, checkoutReference, poisonable,
63+
"Checkout of untrusted code from $@ in a privileged workflow with later potential execution (event trigger: $@).",
64+
checkoutReference, checkoutReferenceText, event, event.getName()

0 commit comments

Comments
 (0)