Skip to content

Commit 8b23a9e

Browse files
woodruffwHolzhaus
andauthored
feat: new audit: forbidden-uses (#664)
Co-authored-by: Jan Holthuis <[email protected]>
1 parent c4600e9 commit 8b23a9e

24 files changed

+531
-105
lines changed

docs/audits.md

Lines changed: 105 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Legend:
1212

1313
| Type | Examples | Introduced in | Works offline | Enabled by default | Configurable |
1414
|----------|------------------|---------------|----------------|--------------------|--------------|
15-
| The kind of audit ("Workflow" or "Action") | Links to vulnerable examples | Added to `zizmor` in this version | The audit works with `--offline` | The audit needs to be explicitly enabled with `--pedantic` | The audit supports custom configuration |
15+
| The kind of audit ("Workflow" or "Action") | Links to vulnerable examples | Added to `zizmor` in this version | The audit works with `--offline` | The audit needs to be explicitly enabled via configuration or an API token | The audit supports custom configuration |
1616

1717
## `artipacked`
1818

@@ -313,7 +313,7 @@ that exists only in a fork can be referenced via its parent's
313313
`owner/repo` slug, and vice versa.
314314

315315
GitHub's network-of-forks design can be used to obscure a commit's true origin
316-
in a fully-pinned `uses:` workflow reference. This can be used by an attacker
316+
in a fully-pinned `#!yaml uses:` workflow reference. This can be used by an attacker
317317
to surreptitiously introduce a backdoored action into a victim's workflows(s).
318318

319319
A notable historical example of this is github/dmca@565ece486c7c1652754d7b6d2b5ed9cb4097f9d5,
@@ -373,7 +373,7 @@ Detects actions that are pinned to confusable symbolic refs (i.e. branches
373373
or tags).
374374

375375
Like with [impostor commits], actions that are used with a symbolic ref
376-
in their `uses:` are subject to a degree of ambiguity: a ref like
376+
in their `#!yaml uses:` are subject to a degree of ambiguity: a ref like
377377
`@v1` might refer to either a branch or tag ref.
378378

379379
An attacker can exploit this ambiguity to publish a branch or tag ref that
@@ -483,7 +483,7 @@ shell quoting/expansion rules.
483483
default shell on Windows runners) uses `${env:VARNAME}`.
484484

485485
To avoid having to specialize your handling for different runners,
486-
you can set `shell: sh` or `shell: bash`.
486+
you can set `#!yaml shell: sh` or `#!yaml shell: bash`.
487487

488488
=== "Before :warning:"
489489

@@ -558,13 +558,13 @@ or @rubygems/release-gem for canonical examples of using it.
558558

559559
[unpinned.yml]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/unpinned.yml
560560

561-
Detects "unpinned" `uses:` clauses.
561+
Detects "unpinned" `#!yaml uses:` clauses.
562562

563-
When a `uses:` clause is not pinned by branch, tag, or SHA reference,
563+
When a `#!yaml uses:` clause is not pinned by branch, tag, or SHA reference,
564564
GitHub Actions will use the latest commit on the referenced repository's
565565
default branch (or, in the case of Docker actions, the `:latest` tag).
566566

567-
Similarly, if a `uses:` clause is pinned via branch or tag (i.e. a "symbolic
567+
Similarly, if a `#!yaml uses:` clause is pinned via branch or tag (i.e. a "symbolic
568568
reference") instead of a SHA reference, GitHub Actions will use whatever
569569
commit is at the tip of that branch or tag. GitHub does not have immutable
570570
branches or tags, meaning that the action can change without the symbolic
@@ -613,34 +613,34 @@ _Type_: `object`
613613
The `rules.unpinned-uses.config.policies` object defines your `unpinned-uses`
614614
policies.
615615

616-
Each member is a `pattern: policy` rule, where `pattern` describes which
617-
`uses:` clauses to match and `policy` describes how to treat them.
616+
Each member is a `#!yaml pattern: policy` rule, where `pattern` describes which
617+
`#!yaml uses:` clauses to match and `policy` describes how to treat them.
618618

619619
The valid patterns are (in order of specificity):
620620

621-
* `owner/repo`: match all `uses:` clauses that are exact matches for the
621+
* `owner/repo`: match all `#!yaml uses:` clauses that are exact matches for the
622622
`owner/repo` pattern.
623623

624624
For example, `actions/checkout` matches only @actions/checkout.
625625

626-
* `owner/*`: match all `uses:` clauses that have the given `owner`.
626+
* `owner/*`: match all `#!yaml uses:` clauses that have the given `owner`.
627627

628628
For example, `actions/*` matches both @actions/checkout and @actions/setup-node.
629629

630-
* `*`: match all `uses:` clauses.
630+
* `*`: match all `#!yaml uses:` clauses.
631631

632632
For example, `*` matches @actions/checkout and @pypa/gh-action-pypi-publish.
633633

634634
The valid policies are:
635635

636-
* `hash-pin`: any `uses:` clauses that match the associated pattern must be
636+
* `hash-pin`: any `#!yaml uses:` clauses that match the associated pattern must be
637637
fully pinned by SHA reference.
638-
* `ref-pin`: any `uses:` clauses that match the associated pattern must be
638+
* `ref-pin`: any `#!yaml uses:` clauses that match the associated pattern must be
639639
pinned either symbolic or SHA reference.
640-
* `any`: no pinning is required for any `uses:` clauses that match the associated
640+
* `any`: no pinning is required for any `#!yaml uses:` clauses that match the associated
641641
pattern.
642642

643-
If a `uses:` clauses matches multiple rules, the most specific one is used
643+
If a `#!yaml uses:` clauses matches multiple rules, the most specific one is used
644644
regardless of definition order. For example, the following
645645
configuration contains two rules that could match @actions/checkout,
646646
but the first one is more specific and therefore gets applied:
@@ -654,10 +654,10 @@ rules:
654654
actions/*: ref-pin
655655
```
656656
657-
In plain English, this policy set says "anything that `uses: actions/*` must
657+
In plain English, this policy set says "anything that `#!yaml uses: actions/*` must
658658
be at least ref-pinned, but @actions/checkout in particular must be hash-pinned."
659659

660-
If a `uses:` clause does not match any rules, then an implicit `"*": hash-pin`
660+
If a `#!yaml uses:` clause does not match any rules, then an implicit `"*": hash-pin`
661661
rule is applied. Users can override this implicit rule by adding their
662662
own `*` rule. For example:
663663

@@ -670,7 +670,7 @@ rules:
670670
"*": ref-pin
671671
```
672672

673-
In plain English, this policy set says "anything that `uses: example/*` must
673+
In plain English, this policy set says "anything that `#!yaml uses: example/*` must
674674
be hash-pinned, and anything else must be at least ref-pinned."
675675

676676
### Remediation
@@ -856,7 +856,7 @@ intended to publish build artifacts:
856856

857857
* Remove cache-aware actions like @actions/cache from workflows that produce
858858
releases, *or*
859-
* Disable cache-aware actions with an `if:` condition based on the trigger at
859+
* Disable cache-aware actions with an `#!yaml if:` condition based on the trigger at
860860
the step level, *or*
861861
* Set an action-specific input to disable cache restoration when appropriate,
862862
such as `lookup-only` in @Swatinem/rust-cache.
@@ -873,14 +873,14 @@ Detects excessive secret inheritance between calling workflows and reusable
873873
(called) workflows.
874874

875875
[Reusable workflows] can be given secrets by their calling workflow either
876-
explicitly, or in a blanket fashion with `secrets: inherit`. The latter
876+
explicitly, or in a blanket fashion with `#!yaml secrets: inherit`. The latter
877877
should almost never be used, as it makes it violates the
878878
[Principle of Least Authority] and makes it impossible to determine which exact
879879
secrets a reusable workflow was executed with.
880880

881881
### Remediation
882882

883-
In general, `secrets: inherit` should be replaced with a `secrets:` block
883+
In general, `#!yaml secrets: inherit` should be replaced with a `#!yaml secrets:` block
884884
that explicitly forwards each secret actually needed by the reusable workflow.
885885

886886
=== "Before :warning:"
@@ -1103,6 +1103,87 @@ individual fields as separate secrets.
11031103
PASSWORD: ${{ secrets.MY_SECRET_PASSWORD }}
11041104
```
11051105

1106+
## `forbidden-uses`
1107+
1108+
| Type | Examples | Introduced in | Works offline | Enabled by default | Configurable |
1109+
|----------|-------------------------|---------------|----------------|--------------------| ---------------|
1110+
| Workflow, Action | N/A | v1.6.0 | ✅ | ❌ | ✅ |
1111+
1112+
An *opt-in* audit for denylisting/allowlisting specific `#!yaml uses:` clauses.
1113+
This is not enabled by default; you must
1114+
[configure it](#forbidden-uses-configuration) to use it.
1115+
1116+
!!! warning
1117+
1118+
This audit comes with several limitations that are important to understand:
1119+
1120+
* This audit is *opt-in*. You must configure it to use it; it
1121+
**does nothing** by default.
1122+
* This audit (currently) operates on *repository* `#!yaml uses:` clauses,
1123+
e.g. `#!yaml uses: actions/checkout@v4`. It does not operate on Docker
1124+
`#!yaml uses:` clauses, e.g. `#!yaml uses: docker://ubuntu:24.04`. This limitation
1125+
may be lifted in the future.
1126+
* This audit operates on `#!yaml uses:` clauses *as they appear* in the workflow
1127+
and action files. In other words, in *cannot* detect
1128+
[impostor commits](#impostor-commit) or indirect usage of actions
1129+
via manual `git clone` and local path usage.
1130+
* This audit's configuration operates on patterns, just like
1131+
[unpinned-uses](#unpinned-uses). That means that you can't (yet)
1132+
define *exact* matches. For example, you can't forbid `actions/checkout@v4`,
1133+
you have to forbid `actions/checkout`, which forbids all versions.
1134+
1135+
### Configuration { #forbidden-uses-configuration }
1136+
1137+
#### `rules.forbidden-uses.config.<allow|deny>`
1138+
1139+
_Type_: `list`
1140+
1141+
The `forbidden-uses` audit operates on either an allowlist or denylist
1142+
basis:
1143+
1144+
* In allowlist mode, only the listed `#!yaml uses:` patterns are allowed. All
1145+
non-matching `#!yaml uses:` clauses result in a finding.
1146+
1147+
Intended use case: only allowing "known good" actions to be used,
1148+
and forbidding everything else.
1149+
1150+
* In denylist mode, only the listed `#!yaml uses:` patterns are disallowed. All
1151+
matching `#!yaml uses:` clauses result in a finding.
1152+
1153+
Intended use case: permitting all `#!yaml uses:` by default, but explicitly
1154+
forbidding "known bad" actions.
1155+
1156+
Regardless of the mode used, the patterns allowed are the same as those
1157+
in [unpinned-uses](#unpinned-uses-configuration).
1158+
1159+
For example, the following configuration would allow only actions owned by
1160+
the @actions organization, plus any actions defined in @github/codeql-action:
1161+
1162+
```yaml title="zizmor.yml"
1163+
rules:
1164+
forbidden-uses:
1165+
config:
1166+
allow:
1167+
- actions/*
1168+
- github/codeql-action
1169+
```
1170+
1171+
Whereas the following would allow all actions except for those in the
1172+
@actions organization or defined in @github/codeql-action:
1173+
1174+
```yaml title="zizmor.yml"
1175+
rules:
1176+
forbidden-uses:
1177+
config:
1178+
deny:
1179+
- actions/*
1180+
- github/codeql-action
1181+
```
1182+
1183+
### Remediation
1184+
1185+
Either remove the offending `#!yaml uses:` clause or, if intended, add it to
1186+
your [configuration](#forbidden-uses-configuration).
11061187

11071188
[ArtiPACKED: Hacking Giants Through a Race Condition in GitHub Actions Artifacts]: https://unit42.paloaltonetworks.com/github-repo-artifacts-leak-tokens/
11081189
[Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests]: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/
@@ -1128,3 +1209,5 @@ individual fields as separate secrets.
11281209
[GitHub Actions exploitations: Dependabot]: https://www.synacktiv.com/publications/github-actions-exploitation-dependabot
11291210
[Using secrets in GitHub Actions]: https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions
11301211
[Palo Alto Networks Unit42: tj-actions/changed-files incident]: https://unit42.paloaltonetworks.com/github-actions-supply-chain-attack/
1212+
1213+

src/audit/forbidden_uses.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
use anyhow::{Context, anyhow};
2+
use github_actions_models::common::Uses;
3+
4+
use super::{Audit, AuditLoadError, AuditState, Finding, Step, audit_meta};
5+
use crate::finding::{Confidence, Persona, Severity};
6+
use crate::models::CompositeStep;
7+
use crate::models::uses::RepositoryUsesPattern;
8+
use serde::Deserialize;
9+
10+
pub(crate) struct ForbiddenUses {
11+
config: ForbiddenUsesConfig,
12+
}
13+
14+
audit_meta!(ForbiddenUses, "forbidden-uses", "forbidden action used");
15+
16+
impl ForbiddenUses {
17+
pub fn use_denied(&self, uses: &Uses) -> bool {
18+
match uses {
19+
// Local uses are never denied.
20+
Uses::Local(_) => false,
21+
// TODO: Support Docker uses here?
22+
// We'd need some equivalent to RepositoryUsesPattern
23+
// but for Docker uses, which will be slightly annoying.
24+
Uses::Docker(_) => {
25+
tracing::warn!("can't evaluate direct Docker uses");
26+
false
27+
}
28+
Uses::Repository(uses) => match &self.config {
29+
ForbiddenUsesConfig::Allow { allow } => {
30+
!allow.iter().any(|pattern| pattern.matches(uses))
31+
}
32+
ForbiddenUsesConfig::Deny { deny } => {
33+
deny.iter().any(|pattern| pattern.matches(uses))
34+
}
35+
},
36+
}
37+
}
38+
}
39+
40+
impl Audit for ForbiddenUses {
41+
fn new(state: &AuditState<'_>) -> Result<Self, AuditLoadError>
42+
where
43+
Self: Sized,
44+
{
45+
let Some(config) = state
46+
.config
47+
.rule_config(Self::ident())
48+
.context("invalid configuration")
49+
.map_err(AuditLoadError::Fail)?
50+
else {
51+
return Err(AuditLoadError::Skip(anyhow!("audit not configured")));
52+
};
53+
54+
Ok(Self { config })
55+
}
56+
57+
fn audit_step<'w>(&self, step: &Step<'w>) -> anyhow::Result<Vec<Finding<'w>>> {
58+
let mut findings = vec![];
59+
60+
let Some(uses) = step.uses() else {
61+
return Ok(vec![]);
62+
};
63+
64+
if self.use_denied(uses) {
65+
findings.push(
66+
Self::finding()
67+
.confidence(Confidence::High)
68+
.severity(Severity::High)
69+
.persona(Persona::Regular)
70+
.add_location(
71+
step.location()
72+
.primary()
73+
.with_keys(&["uses".into()])
74+
.annotated("use of this action is forbidden"),
75+
)
76+
.build(step.workflow())?,
77+
);
78+
};
79+
80+
Ok(findings)
81+
}
82+
83+
fn audit_composite_step<'a>(
84+
&self,
85+
step: &CompositeStep<'a>,
86+
) -> anyhow::Result<Vec<Finding<'a>>> {
87+
let mut findings = vec![];
88+
89+
let Some(uses) = step.uses() else {
90+
return Ok(vec![]);
91+
};
92+
93+
if self.use_denied(uses) {
94+
findings.push(
95+
Self::finding()
96+
.confidence(Confidence::High)
97+
.severity(Severity::High)
98+
.persona(Persona::Regular)
99+
.add_location(
100+
step.location()
101+
.primary()
102+
.with_keys(&["uses".into()])
103+
.annotated("use of this action is forbidden"),
104+
)
105+
.build(step.action())?,
106+
);
107+
};
108+
109+
Ok(findings)
110+
}
111+
}
112+
113+
#[derive(Debug, Deserialize)]
114+
#[serde(rename_all = "kebab-case", untagged)]
115+
enum ForbiddenUsesConfig {
116+
Allow { allow: Vec<RepositoryUsesPattern> },
117+
Deny { deny: Vec<RepositoryUsesPattern> },
118+
}

src/audit/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub(crate) mod bot_conditions;
1818
pub(crate) mod cache_poisoning;
1919
pub(crate) mod dangerous_triggers;
2020
pub(crate) mod excessive_permissions;
21+
pub(crate) mod forbidden_uses;
2122
pub(crate) mod github_env;
2223
pub(crate) mod hardcoded_container_credentials;
2324
pub(crate) mod impostor_commit;

0 commit comments

Comments
 (0)