feat: improve precision for github-env (#199)

woodruffw · Nov 25, 2024 · 15b83bf · 15b83bf
1 parent 34ec67f
commit 15b83bf
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 11 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -25,6 +25,7 @@ moka = { version = "0.12.8", features = ["sync"] }
 owo-colors = "4.1.0"
 pest = "2.7.14"
 pest_derive = "2.7.14"
+regex = "1.11.1"
 reqwest = { version = "0.12.9", features = ["blocking", "json"] }
 serde = { version = "1.0.215", features = ["derive"] }
 serde-sarif = "0.6.5"

diff --git a/src/audit/github_env.rs b/src/audit/github_env.rs
@@ -3,19 +3,27 @@ use crate::finding::{Confidence, Finding, Severity};
 use crate::models::Step;
 use crate::state::AuditState;
 use github_actions_models::workflow::job::StepBody;
+use regex::RegexSet;
 use std::ops::Deref;
+use std::sync::LazyLock;
+
+static GITHUB_ENV_WRITE_SHELL: LazyLock<RegexSet> = LazyLock::new(|| {
+    RegexSet::new([
+        // matches the `... >> $GITHUB_ENV` pattern
+        r#"(?m)^.+\s*>>?\s*"?\$\{?GITHUB_ENV\}?"?.*$"#,
+        // matches the `... | tee $GITHUB_ENV` pattern
+        r#"(?m)^.*\|\s*tee\s+"?\$\{?GITHUB_ENV\}?"?.*$"#,
+    ])
+    .unwrap()
+});
 
 pub(crate) struct GitHubEnv;
 
-audit_meta!(GitHubEnv, "github-env", "dangerous use of $GITHUB_ENV");
+audit_meta!(GitHubEnv, "github-env", "dangerous use of GITHUB_ENV");
 
 impl GitHubEnv {
-    fn uses_github_environment(&self, run_step_body: &str) -> bool {
-        // In the future we can improve over this implementation,
-        // eventually detecting how $GITHUB_ENV is being used
-        // and returning an Option<Confidence> instead
-
-        run_step_body.contains("$GITHUB_ENV") || run_step_body.contains("${GITHUB_ENV}")
+    fn uses_github_environment(run_step_body: &str) -> bool {
+        GITHUB_ENV_WRITE_SHELL.is_match(run_step_body)
     }
 }
 
@@ -40,14 +48,16 @@ impl WorkflowAudit for GitHubEnv {
         }
 
         if let StepBody::Run { run, .. } = &step.deref().body {
-            if self.uses_github_environment(run) {
+            if Self::uses_github_environment(run) {
                 findings.push(
                     Self::finding()
                         .severity(Severity::High)
                         .confidence(Confidence::Low)
-                        .add_location(step.location().with_keys(&["run".into()]).annotated(
-                            "GITHUB_ENV used in the context of a dangerous Workflow trigger",
-                        ))
+                        .add_location(
+                            step.location()
+                                .with_keys(&["run".into()])
+                                .annotated("GITHUB_ENV write may allow code execution"),
+                        )
                         .build(step.workflow())?,
                 )
             }
@@ -56,3 +66,39 @@ impl WorkflowAudit for GitHubEnv {
         Ok(findings)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::audit::github_env::GitHubEnv;
+
+    #[test]
+    fn test_shell_patterns() {
+        for case in &[
+            // Common cases
+            "echo foo >> $GITHUB_ENV",
+            "echo foo >> \"$GITHUB_ENV\"",
+            "echo foo >> ${GITHUB_ENV}",
+            "echo foo >> \"${GITHUB_ENV}\"",
+            // Single > is buggy most of the time, but still exploitable
+            "echo foo > $GITHUB_ENV",
+            "echo foo > \"$GITHUB_ENV\"",
+            "echo foo > ${GITHUB_ENV}",
+            "echo foo > \"${GITHUB_ENV}\"",
+            // No spaces
+            "echo foo>>$GITHUB_ENV",
+            "echo foo>>\"$GITHUB_ENV\"",
+            "echo foo>>${GITHUB_ENV}",
+            "echo foo>>\"${GITHUB_ENV}\"",
+            // tee cases
+            "something | tee $GITHUB_ENV",
+            "something | tee \"$GITHUB_ENV\"",
+            "something | tee ${GITHUB_ENV}",
+            "something | tee \"${GITHUB_ENV}\"",
+            "something|tee $GITHUB_ENV",
+            "something |tee $GITHUB_ENV",
+            "something| tee $GITHUB_ENV",
+        ] {
+            assert!(GitHubEnv::uses_github_environment(case));
+        }
+    }
+}