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: detects well-known publisher actions in cache-audit #338

Merged
merged 3 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 104 additions & 27 deletions src/audit/cache_poisoning.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::audit::{audit_meta, WorkflowAudit};
use crate::finding::{Confidence, Finding, Severity};
use crate::models::{Step, Uses};
use crate::models::{Job, Step, Steps, Uses};
use crate::state::AuditState;
use github_actions_models::common::expr::ExplicitExpr;
use github_actions_models::common::Env;
Expand Down Expand Up @@ -110,13 +110,30 @@ static KNOWN_CACHE_AWARE_ACTIONS: LazyLock<Vec<CacheAwareAction>> = LazyLock::ne
]
});

/// A list of well-know publisher actions
/// In the future we can retrieve this list from the static API
static KNOWN_PUBLISHER_ACTIONS: LazyLock<Vec<Uses>> = LazyLock::new(|| {
vec![
Uses::from_step("pypa/gh-action-pypi-publish").unwrap(),
Uses::from_step("softprops/action-gh-release").unwrap(),
woodruffw marked this conversation as resolved.
Show resolved Hide resolved
Uses::from_step("rubygems/release-gem").unwrap(),
Uses::from_step("goreleaser/goreleaser-action").unwrap(),
Uses::from_step("jreleaser/release-action").unwrap(),
]
});

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

enum PublishingArtifactsScenario<'w> {
UsingTypicalWorkflowTrigger,
UsingWellKnowPublisherAction(Step<'w>),
}

pub(crate) struct CachePoisoning;

audit_meta!(
Expand All @@ -137,6 +154,38 @@ impl CachePoisoning {
}
}

fn detected_well_known_publisher_step(steps: Steps) -> Option<Step> {
steps.into_iter().find(|step| {
let Some(Uses::Repository(target_uses)) = step.uses() else {
return false;
};

KNOWN_PUBLISHER_ACTIONS.iter().any(|publisher| {
let Uses::Repository(well_known_uses) = publisher else {
return false;
};

target_uses.matches(*well_known_uses)
})
})
}

fn is_job_publishing_artifacts<'w>(
&self,
trigger: &Trigger,
steps: Steps<'w>,
) -> Option<PublishingArtifactsScenario<'w>> {
if self.trigger_used_when_publishing_artifacts(trigger) {
return Some(PublishingArtifactsScenario::UsingTypicalWorkflowTrigger);
};

let well_know_publisher = CachePoisoning::detected_well_known_publisher_step(steps)?;

Some(PublishingArtifactsScenario::UsingWellKnowPublisherAction(
well_know_publisher,
))
}

fn evaluate_default_action_behaviour(action: &CacheAwareAction) -> Option<CacheUsage> {
if action.caching_by_default {
Some(CacheUsage::DefaultActionBehaviour)
Expand Down Expand Up @@ -218,41 +267,26 @@ impl CachePoisoning {
}
}
}
}

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) {
return Ok(findings);
}

fn uses_cache_aware_step<'w>(
&self,
step: &Step<'w>,
scenario: &PublishingArtifactsScenario<'w>,
) -> Option<Finding<'w>> {
let StepBody::Uses { ref uses, ref with } = &step.deref().body else {
return Ok(findings);
return None;
};

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

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()
let finding = match scenario {
PublishingArtifactsScenario::UsingTypicalWorkflowTrigger => Self::finding()
.confidence(Confidence::Low)
.severity(Severity::High)
.add_location(
Expand All @@ -267,8 +301,51 @@ impl WorkflowAudit for CachePoisoning {
.with_keys(&[yaml_key.into()])
.annotated(annotation),
)
.build(step.workflow())?,
);
.build(step.workflow()),
PublishingArtifactsScenario::UsingWellKnowPublisherAction(publisher) => Self::finding()
.confidence(Confidence::Low)
.severity(Severity::High)
.add_location(
publisher
.location()
.with_keys(&["uses".into()])
.annotated("runtime artifacts usually published here"),
)
.add_location(
step.location()
.primary()
.with_keys(&[yaml_key.into()])
.annotated(annotation),
)
.build(step.workflow()),
};

finding.ok()
}
}

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

fn audit_normal_job<'w>(&self, job: &Job<'w>) -> anyhow::Result<Vec<Finding<'w>>> {
let mut findings = vec![];
let steps = job.steps();
let trigger = &job.parent().on;

let Some(scenario) = self.is_job_publishing_artifacts(trigger, steps) else {
return Ok(findings);
};

for step in job.steps() {
if let Some(finding) = self.uses_cache_aware_step(&step, &scenario) {
findings.push(finding);
}
}

Ok(findings)
}
Expand Down
4 changes: 4 additions & 0 deletions tests/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,5 +314,9 @@ fn cache_poisoning() -> Result<()> {
))
.run()?);

insta::assert_snapshot!(zizmor()
.workflow(workflow_under_test("cache-poisoning/publisher-step.yml"))
.run()?);

Ok(())
}
19 changes: 19 additions & 0 deletions tests/snapshots/snapshot__cache_poisoning-10.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
source: tests/snapshot.rs
expression: "zizmor().workflow(workflow_under_test(\"cache-poisoning/publisher-step.yml\")).run()?"
snapshot_kind: text
---
error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack
--> @@INPUT@@:23:9
|
16 | uses: Swatinem/rust-cache@82a92a6e8fbeee089604da2575dc567ae9ddeaab
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cache enabled by default here
17 |
...
22 | - name: Publish draft release on Github
23 | uses: softprops/action-gh-release@01570a1f39cb168c169c802c3bceb9e93fb10974
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ runtime artifacts usually published here
|
= note: audit confidence → Low

1 finding: 0 unknown, 0 informational, 0 low, 0 medium, 1 high
32 changes: 32 additions & 0 deletions tests/test-data/cache-poisoning/publisher-step.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
on:
push:
branches:
- main

jobs:
publish:
runs-on: macos-latest
steps:
- name: Project Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
with:
persist-credentials: false

- name: Setup CI caching
uses: Swatinem/rust-cache@82a92a6e8fbeee089604da2575dc567ae9ddeaab

- name: Build snapshot version of artifacts
id: build
run: cargo xtasks darwin-build

- name: Publish draft release on Github
uses: softprops/action-gh-release@01570a1f39cb168c169c802c3bceb9e93fb10974
with:
name: ${{ steps.build.outputs.version }}
tag_name: ${{ steps.build.outputs.version }}
token: ${{ secrets.GITHUB_RELEASES_TOKEN }}
generate_release_notes: false
draft: false
files: |
target/aarch64-apple-darwin/release/my-app
target/x86_64-apple-darwin/release/my-app
Loading