Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: initial cache-poisoning audit #294

Merged
merged 13 commits into from
Dec 19, 2024
49 changes: 49 additions & 0 deletions docs/audits.md
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,54 @@ like `pull_request_target`.

If you need to pass state between steps, consider using `GITHUB_OUTPUT` instead.

## `cache-poisoning`

| Type | Examples | Introduced in | Works offline | Enabled by default |
|----------|-------------------------|---------------|----------------|--------------------|
| Workflow | [cache-poisoning.yml] | v0.10.0 | ✅ | ✅ |

[cache-poisoning.yml]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/cache-poisoning.yml

Detects potential cache-poisoning scenarios in release workflows.

Caching and restoring build state is a process eased by utilities provided
by GitHub, in particular @actions/cache and its "save" and "restore"
sub-actions. In addition, many of the setup-like actions provided
by GitHub come with built-in caching functionality, like @actions/setup-node,
@actions/setup-java and others.

Furthermore, there are many examples of community-driven Actions with built-in
caching functionality, like @ruby/setup-ruby, @astral-sh/setup-uv,
@Swatinem/rust-cache. In general, most of them build on top of @actions/tookit
for the sake of easily integrate with GitHub cache server at Workflow runtime.

This vulnerability happens when release workflows leverage build state cached
from previous workflow executions, in general on top of the aforementioned
actions or similar ones. The publication of artifacts usually happens driven
by trigger events like `release` or events with path filters like `push`
(e.g. for tags).

In such scenarios, an attacker with access to a valid `GITHUB_TOKEN` can use it
poison the repository's GitHub Actions caches. That compounds with the default
behavior of @actions/tookit during cache restorations, allowing an attacker to
retrieve payloads from poisoned cache entries, hence achieving code execution at
Workflow runtime, potentially compromising ready-to-publish artifacts.

Other resources:

* [The Monsters in Your Build Cache – GitHub Actions Cache Poisoning]

### Remediation

In general, you should avoid using previously cached CI state within workflows
intended to publish build artifacts:

* Remove cache-aware actions like @actions/cache from workflows that produce
releases, *or*
* Disable cache-aware actions with an `if:` condition based on the trigger at
the step level, *or*
* Set an action-specific input to disable cache restoration when appropriate,
such as `lookup-only` in @Swatinem/rust-cache.

[ArtiPACKED: Hacking Giants Through a Race Condition in GitHub Actions Artifacts]: https://unit42.paloaltonetworks.com/github-repo-artifacts-leak-tokens/
[Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests]: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/
Expand All @@ -711,3 +759,4 @@ If you need to pass state between steps, consider using `GITHUB_OUTPUT` instead.
[Vulnerable GitHub Actions Workflows Part 1: Privilege Escalation Inside Your CI/CD Pipeline]: https://www.legitsecurity.com/blog/github-privilege-escalation-vulnerability
[Google & Apache Found Vulnerable to GitHub Environment Injection]: https://www.legitsecurity.com/blog/github-privilege-escalation-vulnerability-0
[Hacking with Environment Variables]: https://www.elttam.com/blog/env/
[The Monsters in Your Build Cache – GitHub Actions Cache Poisoning]: https://adnanthekhan.com/2024/05/06/the-monsters-in-your-build-cache-github-actions-cache-poisoning/
275 changes: 275 additions & 0 deletions src/audit/cache_poisoning.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,275 @@
use crate::audit::{audit_meta, WorkflowAudit};
use crate::finding::{Confidence, Finding, Severity};
use crate::models::{Step, Uses};
use crate::state::AuditState;
use github_actions_models::common::expr::ExplicitExpr;
use github_actions_models::common::Env;
use github_actions_models::workflow::event::{BareEvent, OptionalBody};
use github_actions_models::workflow::job::StepBody;
use github_actions_models::workflow::Trigger;
use std::ops::Deref;
use std::sync::LazyLock;

#[derive(PartialEq)]
enum ControlValue {
Boolean,
String,
}

enum CacheControl {
OptIn(&'static str),
OptOut(&'static str),
}

/// The general schema for a cache-aware actions
struct CacheAwareAction<'w> {
/// The owner/repo part within the Action full coordinate
uses: Uses<'w>,
/// The input that controls caching behavior
cache_control: CacheControl,
/// The type of value used to opt-in/opt-out (Boolean, String)
control_value: ControlValue,
/// Whether this Action adopts caching as the default behavior
caching_by_default: bool,
}

/// The list of know cache-aware actions
/// In the future we can easily retrieve this list from the static API,
/// since it should be easily serializable
static KNOWN_CACHE_AWARE_ACTIONS: LazyLock<Vec<CacheAwareAction>> = LazyLock::new(|| {
ubiratansoares marked this conversation as resolved.
Show resolved Hide resolved
vec![
ubiratansoares marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/actions/cache/blob/main/action.yml
CacheAwareAction {
uses: Uses::from_step("actions/cache").unwrap(),
cache_control: CacheControl::OptOut("lookup-only"),
control_value: ControlValue::Boolean,
caching_by_default: true,
},
CacheAwareAction {
uses: Uses::from_step("actions/setup-java").unwrap(),
cache_control: CacheControl::OptIn("cache"),
control_value: ControlValue::String,
caching_by_default: false,
},
// https://github.com/actions/setup-go/blob/main/action.yml
CacheAwareAction {
uses: Uses::from_step("actions/setup-go").unwrap(),
cache_control: CacheControl::OptIn("cache"),
control_value: ControlValue::String,
caching_by_default: true,
},
// https://github.com/actions/setup-node/blob/main/action.yml
CacheAwareAction {
uses: Uses::from_step("actions/setup-node").unwrap(),
cache_control: CacheControl::OptIn("cache"),
control_value: ControlValue::String,
caching_by_default: false,
},
// https://github.com/actions/setup-python/blob/main/action.yml
CacheAwareAction {
uses: Uses::from_step("actions/setup-python").unwrap(),
cache_control: CacheControl::OptIn("cache"),
control_value: ControlValue::String,
caching_by_default: false,
},
// https://github.com/actions/setup-dotnet/blob/main/action.yml
CacheAwareAction {
uses: Uses::from_step("actions/setup-dotnet").unwrap(),
cache_control: CacheControl::OptIn("cache"),
control_value: ControlValue::Boolean,
caching_by_default: false,
},
// https://github.com/astral-sh/setup-uv/blob/main/action.yml
CacheAwareAction {
uses: Uses::from_step("astral-sh/setup-uv").unwrap(),
cache_control: CacheControl::OptOut("enable-cache"),
control_value: ControlValue::String,
caching_by_default: true,
},
// https://github.com/Swatinem/rust-cache/blob/master/action.yml
CacheAwareAction {
uses: Uses::from_step("Swatinem/rust-cache").unwrap(),
cache_control: CacheControl::OptOut("lookup-only"),
control_value: ControlValue::Boolean,
caching_by_default: true,
},
// https://github.com/ruby/setup-ruby/blob/master/action.yml
CacheAwareAction {
uses: Uses::from_step("ruby/setup-ruby").unwrap(),
cache_control: CacheControl::OptIn("bundler-cache"),
control_value: ControlValue::Boolean,
caching_by_default: false,
},
// https://github.com/PyO3/maturin-action/blob/main/action.yml
CacheAwareAction {
uses: Uses::from_step("PyO3/maturin-action").unwrap(),
cache_control: CacheControl::OptIn("sccache"),
control_value: ControlValue::Boolean,
caching_by_default: false,
},
]
});

#[derive(PartialEq)]
enum CacheUsage {
ConditionalOptIn,
DirectOptIn,
DefaultActionBehaviour,
}

pub(crate) struct CachePoisoning;

audit_meta!(
CachePoisoning,
"cache-poisoning",
"runtime artifacts potentially vulnerable to a cache poisoning attack"
);

impl CachePoisoning {
fn trigger_used_when_publishing_artifacts(&self, trigger: &Trigger) -> bool {
match trigger {
Trigger::BareEvent(event) => *event == BareEvent::Release,
Trigger::BareEvents(events) => events.contains(&BareEvent::Release),
Trigger::Events(events) => match &events.push {
OptionalBody::Body(body) => body.tag_filters.is_some(),
_ => false,
},
}
}

fn evaluate_default_action_behaviour(action: &CacheAwareAction) -> Option<CacheUsage> {
if action.caching_by_default {
Some(CacheUsage::DefaultActionBehaviour)
} else {
None
}
}

fn evaluate_user_defined_opt_in(
cache_control_input: &str,
env: &Env,
action: &CacheAwareAction,
) -> Option<CacheUsage> {
match env.get(cache_control_input) {
None => None,
Some(value) => match value.to_string().as_str() {
"true" if matches!(action.control_value, ControlValue::Boolean) => {
Some(CacheUsage::DirectOptIn)
}
"false" if matches!(action.control_value, ControlValue::Boolean) => {
// Explicitly opts out from caching
None
}
other => match ExplicitExpr::from_curly(other) {
None if matches!(action.control_value, ControlValue::String) => {
Some(CacheUsage::DirectOptIn)
}
None => None,
Some(_) => Some(CacheUsage::ConditionalOptIn),
},
},
}
}

fn evaluate_cache_usage(&self, target_step: &str, env: &Env) -> Option<CacheUsage> {
let known_action = KNOWN_CACHE_AWARE_ACTIONS.iter().find(|action| {
let Uses::Repository(well_known_uses) = action.uses else {
return false;
};

let Some(Uses::Repository(target_uses)) = Uses::from_step(target_step) else {
return false;
};

target_uses.matches(well_known_uses)
})?;

let cache_control_input = env.keys().find(|k| match known_action.cache_control {
CacheControl::OptIn(inner) => *k == inner,
CacheControl::OptOut(inner) => *k == inner,
});

match cache_control_input {
// when not using the specific Action input to control caching behaviour,
// we evaluate whether it uses caching by default
None => CachePoisoning::evaluate_default_action_behaviour(known_action),

// otherwise, we infer from the value assigned to the cache control input
Some(key) => {
// first, we extract the value assigned to that input
let declared_usage =
CachePoisoning::evaluate_user_defined_opt_in(key, env, known_action);

// we now evaluate the extracted value against the opt-in semantics
match &declared_usage {
Some(CacheUsage::DirectOptIn) => {
match known_action.cache_control {
// in this case, we just follow the opt-in
CacheControl::OptIn(_) => declared_usage,
// otherwise, the user opted for disabling the cache
// hence we don't return a CacheUsage
CacheControl::OptOut(_) => None,
}
}
// Because we can't evaluate expressions, there is nothing to do
// regarding CacheUsage::ConditionalOptIn
_ => declared_usage,
}
}
}
}
}

impl WorkflowAudit for CachePoisoning {
fn new(_: AuditState) -> anyhow::Result<Self>
where
Self: Sized,
{
Ok(Self)
}

fn audit_step<'w>(&self, step: &Step<'w>) -> anyhow::Result<Vec<Finding<'w>>> {
let mut findings = vec![];

let trigger = &step.workflow().on;

if !self.trigger_used_when_publishing_artifacts(trigger) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good for now, thanks!

Thinking out loud: another future proxy we could use for "is this a publishing workflow" is whether or not the workflow contains any well-known publishing actions, e.g. gh-action-pypi-publish.

Not something to do with this iteration, but worth thinking about 🙂

return Ok(findings);
}

let StepBody::Uses { ref uses, ref with } = &step.deref().body else {
return Ok(findings);
};

let Some(cache_usage) = self.evaluate_cache_usage(uses, with) else {
return Ok(findings);
};

let (yaml_key, annotation) = match cache_usage {
CacheUsage::DefaultActionBehaviour => ("uses", "cache enabled by default here"),
CacheUsage::DirectOptIn => ("with", "opt-in for caching here"),
CacheUsage::ConditionalOptIn => ("with", "opt-in for caching might happen here"),
};

findings.push(
Self::finding()
.confidence(Confidence::Low)
.severity(Severity::High)
ubiratansoares marked this conversation as resolved.
Show resolved Hide resolved
.add_location(
step.workflow()
.location()
.with_keys(&["on".into()])
.annotated("generally used when publishing artifacts generated at runtime"),
)
.add_location(
step.location()
.primary()
.with_keys(&[yaml_key.into()])
.annotated(annotation),
)
.build(step.workflow())?,
);

Ok(findings)
}
}
1 change: 1 addition & 0 deletions src/audit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::{
};

pub(crate) mod artipacked;
pub(crate) mod cache_poisoning;
pub(crate) mod dangerous_triggers;
pub(crate) mod excessive_permissions;
pub(crate) mod github_env;
Expand Down
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ fn run() -> Result<ExitCode> {
register_audit!(audit::unpinned_uses::UnpinnedUses);
register_audit!(audit::insecure_commands::InsecureCommands);
register_audit!(audit::github_env::GitHubEnv);
register_audit!(audit::cache_poisoning::CachePoisoning);

let mut results = FindingRegistry::new(&app, &config);
{
Expand Down
18 changes: 15 additions & 3 deletions src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use crate::finding::{Route, SymbolicLocation};
use crate::registry::WorkflowKey;
use crate::utils::extract_expressions;
use anyhow::{bail, Context, Result};
use anyhow::{anyhow, bail, Context, Result};
use camino::Utf8Path;
use github_actions_models::common::expr::LoE;
use github_actions_models::workflow::event::{BareEvent, OptionalBody};
Expand Down Expand Up @@ -599,14 +599,26 @@ pub(crate) struct RepositoryUses<'a> {
pub(crate) git_ref: Option<&'a str>,
}

impl<'a> TryFrom<&'a str> for RepositoryUses<'a> {
type Error = anyhow::Error;

fn try_from(value: &'a str) -> std::result::Result<Self, Self::Error> {
let Some(Uses::Repository(uses)) = Uses::from_common(value) else {
return Err(anyhow!("invalid repository uses: {value}"));
};

Ok(uses)
}
}

impl RepositoryUses<'_> {
/// Returns whether this `uses:` clause "matches" the given template.
/// The template is itself formatted like a normal `uses:` clause.
///
/// This is an asymmetrical match: `actions/checkout@v3` "matches"
/// the `actions/checkout` template but not vice versa.
pub(crate) fn matches(&self, template: &str) -> bool {
let Some(Uses::Repository(other)) = Uses::from_step(template) else {
pub(crate) fn matches<'a>(&self, template: impl TryInto<RepositoryUses<'a>>) -> bool {
let Ok(other) = template.try_into() else {
return false;
};

Expand Down
Loading
Loading