Skip to content

Commit

Permalink
reuse expression handling
Browse files Browse the repository at this point in the history
  • Loading branch information
woodruffw committed Aug 28, 2024
1 parent cb5cd3e commit a96b971
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 19 deletions.
34 changes: 15 additions & 19 deletions src/audit/template_injection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,49 +8,48 @@
use std::ops::Deref;

use github_actions_models::workflow::{job::StepBody, Job};
use regex::Regex;

use crate::{
finding::{Confidence, Severity},
models::AuditConfig,
utils::iter_expressions,
};

use super::WorkflowAudit;

pub(crate) struct TemplateInjection<'a> {
pub(crate) _config: AuditConfig<'a>,
expr_pattern: Regex,
}

impl<'a> TemplateInjection<'a> {
fn injectable_template_expressions<'expr>(
fn injectable_template_expressions(
&self,
run: &'expr str,
) -> Vec<(&'expr str, Severity, Confidence)> {
run: &str,
) -> Vec<(String, Severity, Confidence)> {
let mut bad_expressions = vec![];
for (_, [expr]) in self.expr_pattern.captures_iter(run).map(|c| c.extract()) {
log::debug!("found expression candidate: {expr}");
for expr in iter_expressions(run) {
let bare = expr.as_bare();

if expr.starts_with("secrets.") {
if bare.starts_with("secrets.") {
// While not ideal, secret expansion is typically not exploitable.
continue;
} else if expr.starts_with("inputs.") {
} else if bare.starts_with("inputs.") {
// TODO: Currently low confidence because we don't check the
// input's type. In the future, we should index back into
// the workflow's triggers and exclude input expansions
// from innocuous types, e.g. booleans.
bad_expressions.push((expr, Severity::High, Confidence::Low));
} else if expr.starts_with("env.") {
bad_expressions.push((bare.into(), Severity::High, Confidence::Low));
} else if bare.starts_with("env.") {
// Almost never exploitable.
bad_expressions.push((expr, Severity::Low, Confidence::High));
} else if expr.starts_with("github.event.") {
bad_expressions.push((bare.into(), Severity::Low, Confidence::High));
} else if bare.starts_with("github.event.") {
// TODO: Filter these more finely; not everything in the event
// context is actually attacker-controllable.
bad_expressions.push((expr, Severity::High, Confidence::High));
bad_expressions.push((bare.into(), Severity::High, Confidence::High));
} else {
// All other contexts are typically not attacker controllable,
// but may be in obscure cases.
bad_expressions.push((expr, Severity::Informational, Confidence::Low));
bad_expressions.push((bare.into(), Severity::Informational, Confidence::Low));
}
}

Expand All @@ -70,10 +69,7 @@ impl<'a> WorkflowAudit<'a> for TemplateInjection<'a> {
where
Self: Sized,
{
Ok(Self {
_config: config,
expr_pattern: Regex::new("\\$\\{\\{\\s*(.+)\\s*\\}\\}").unwrap(),
})
Ok(Self { _config: config })
}

fn audit<'w>(
Expand Down
19 changes: 19 additions & 0 deletions src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
//! Helper routines.
use std::sync::LazyLock;

use github_actions_models::common::Expression;
use regex::Regex;

static EXPRESSION_RE: LazyLock<Regex> =
LazyLock::new(|| Regex::new("(\\$\\{\\{.+\\}\\})").unwrap());

/// Splits the given `patterns` string into one or more patterns, using
/// approximately the same rules as GitHub's `@actions/glob` package.
pub(crate) fn split_patterns(patterns: &str) -> impl Iterator<Item = &str> {
Expand All @@ -14,6 +22,17 @@ pub(crate) fn split_patterns(patterns: &str) -> impl Iterator<Item = &str> {
.filter(|line| !line.is_empty() && !line.starts_with('#'))
}

/// Yields each expression in a free-form string.
///
/// This is typically useful for string inputs to actions and
/// `run:` sections.
pub(crate) fn iter_expressions(text: &str) -> impl Iterator<Item = Expression> + '_ {
EXPRESSION_RE.find_iter(text).map(|m| {
Expression::from_curly(m.as_str().to_string())
.expect("impossible: regex does not satisfy expression pattern")
})
}

#[cfg(test)]
mod tests {
#[test]
Expand Down

0 comments on commit a96b971

Please sign in to comment.