Repository: woodruffw/zizmor (language: python)

Issue:
New audit: `ACTIONS_ALLOW_UNSECURE_COMMANDS`
Setting this anywhere in the environment re-enables GHA's old `::set-env` command, which makes code injection via the environment much easier.

Ref: https://semgrep.dev/r?q=yaml.github-actions.security.allowed-unsecure-commands.allowed-unsecure-commands
New audit: `ACTIONS_ALLOW_UNSECURE_COMMANDS`
Setting this anywhere in the environment re-enables GHA's old `::set-env` command, which makes code injection via the environment much easier.

Ref: https://semgrep.dev/r?q=yaml.github-actions.security.allowed-unsecure-commands.allowed-unsecure-commands


Discussion / hints:
Hey @woodruffw ! I'd love to contributing with a new audit, this looks like an easy one to get started! 😄

Ooc, which criticality would you assign to this audit*?
Thanks @ubiratansoares! It's all yours to work on.

I think this should be "high" severity: it opens up a well-known security hole that's been disabled by default for years.

Hey @woodruffw ! I'd love to contributing with a new audit, this looks like an easy one to get started! 😄

Ooc, which criticality would you assign to this audit*?
Thanks @ubiratansoares! It's all yours to work on.

I think this should be "high" severity: it opens up a well-known security hole that's been disabled by default for years.


Fix patch:
diff --git a/.gitignore b/.gitignore
index d1a4af9..5c6beed 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@
 
 # website artifacts
 /site_html
+.cache
 
 # IDEs / Editors
 .idea
diff --git a/docs/audits.md b/docs/audits.md
index c545f9a..a334023 100644
--- a/docs/audits.md
+++ b/docs/audits.md
@@ -592,6 +592,52 @@ A before/after example is shown below.
     1. Or `actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683` for a SHA-pinned action.
 
 
+## `insecure-commands`
+
+| Type     | Examples                | Introduced in | Works offline  | Enabled by default |
+|----------|-------------------------|---------------|----------------|--------------------|
+| Workflow  | [insecure-commands.yml] | v0.5.0        | ✅             | ✅                 |
+
+[insecure-commands.yml]: https://github.com/woodruffw/gha-hazmat/blob/main/.github/workflows/insecure-commands.yml
+
+Detects opt-in for executing insecure workflow commands.
+
+Workflow commands (like `::set-env` and `::add-path`)
+[were deprecated by Github] in 2020 due to their inherent weaknesses
+(e.g., allowing any command with the ability to emit to `stdout`
+to inject environment variables and therefore obtain code execution).
+
+However, users can explicitly re-enable them by setting the
+`ACTIONS_ALLOW_UNSECURE_COMMANDS` environment variable at the workflow,
+job, or step level.
+
+Other resources:
+
+* [Semgrep audit]
+
+### Remediation
+
+In general, users should use for [Github Actions environment files]
+(like `GITHUB_PATH` and `GITHUB_OUTPUT`) instead of using workflow commands.
+
+=== "Before"
+
+    ```yaml title="insecure-commands" hl_lines="3"
+    - name: Setup my-bin
+      run: |
+        echo "::add-path::$HOME/.local/my-bin"
+      env:
+        ACTIONS_ALLOW_UNSECURE_COMMANDS: true
+    ```
+
+=== "After"
+
+    ```yaml title="insecure-commands" hl_lines="3"
+    - name: Setup my-bin
+      run: |
+        echo "$HOME/.local/my-bin" >> "$GITHUB_PATH"
+    ```
+
 [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/
 [What the fork? Imposter commits in GitHub Actions and CI/CD]: https://www.chainguard.dev/unchained/what-the-fork-imposter-commits-in-github-actions-and-ci-cd
@@ -601,3 +647,6 @@ A before/after example is shown below.
 [Trusted Publishing - RubyGems Guides]: https://guides.rubygems.org/trusted-publishing/
 [Trusted publishing: a new benchmark for packaging security]: https://blog.trailofbits.com/2023/05/23/trusted-publishing-a-new-benchmark-for-packaging-security/
 [Trusted Publishers for All Package Repositories]: https://repos.openssf.org/trusted-publishers-for-all-package-repositories.html
+[were deprecated by Github]: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/
+[Github Actions environment files]: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#environment-files
+[Semgrep audit]: https://semgrep.dev/r?q=yaml.github-actions.security.allowed-unsecure-commands.allowed-unsecure-commands
diff --git a/src/audit/insecure_commands.rs b/src/audit/insecure_commands.rs
new file mode 100644
index 0000000..6653e1f
--- /dev/null
+++ b/src/audit/insecure_commands.rs
@@ -0,0 +1,103 @@
+use crate::audit::WorkflowAudit;
+use crate::finding::{Confidence, Finding, FindingBuilder, Severity, SymbolicLocation};
+use crate::models::{Steps, Workflow};
+use crate::state::AuditState;
+use github_actions_models::common::{Env, EnvValue};
+use github_actions_models::workflow::job::StepBody;
+use github_actions_models::workflow::Job;
+use std::ops::Deref;
+
+static ID: &str = "insecure-commands";
+static DESCRIPTION: &str = "execution of insecure workflow commands is enabled";
+
+pub(crate) struct InsecureCommands;
+
+impl InsecureCommands {
+    fn insecure_commands_allowed<'w>(
+        &self,
+        workflow: &'w Workflow,
+        location: SymbolicLocation<'w>,
+    ) -> Finding<'w> {
+        FindingBuilder::new(ID, DESCRIPTION)
+            .confidence(Confidence::High)
+            .severity(Severity::High)
+            .add_location(
+                location
+                    .with_keys(&["env".into()])
+                    .annotated("insecure commands enabled here"),
+            )
+            .build(workflow)
+            .expect("Cannot build a Finding instance")
+    }
+
+    fn has_insecure_commands_enabled(&self, env: &Env) -> bool {
+        if let Some(EnvValue::String(value)) = env.get("ACTIONS_ALLOW_UNSECURE_COMMANDS") {
+            !value.is_empty()
+        } else {
+            false
+        }
+    }
+
+    fn audit_steps<'w>(&self, workflow: &'w Workflow, steps: Steps<'w>) -> Vec<Finding<'w>> {
+        steps
+            .into_iter()
+            .filter(|step| {
+                let StepBody::Run {
+                    run: _,
+                    working_directory: _,
+                    shell: _,
+                    ref env,
+                } = &step.deref().body
+                else {
+                    return false;
+                };
+
+                self.has_insecure_commands_enabled(env)
+            })
+            .map(|step| self.insecure_commands_allowed(workflow, step.location()))
+            .collect()
+    }
+}
+
+impl WorkflowAudit for InsecureCommands {
+    fn ident() -> &'static str
+    where
+        Self: Sized,
+    {
+        ID
+    }
+
+    fn desc() -> &'static str
+    where
+        Self: Sized,
+    {
+        DESCRIPTION
+    }
+
+    fn new(_: AuditState) -> anyhow::Result<Self>
+    where
+        Self: Sized,
+    {
+        Ok(Self {})
+    }
+
+    fn audit<'w>(&self, workflow: &'w Workflow) -> anyhow::Result<Vec<Finding<'w>>> {
+        let mut results = vec![];
+
+        if self.has_insecure_commands_enabled(&workflow.env) {
+            results.push(self.insecure_commands_allowed(workflow, workflow.location()))
+        }
+
+        for job in workflow.jobs() {
+            if let Job::NormalJob(normal) = *job {
+                if self.has_insecure_commands_enabled(&normal.env) {
+                    results.push(self.insecure_commands_allowed(workflow, job.location()))
+                }
+
+                results.extend(self.audit_steps(workflow, job.steps()))
+            }
+        }
+
+        Ok(results)
+    }
+}
diff --git a/src/audit/mod.rs b/src/audit/mod.rs
index 9fca985..0e6f22e 100644
--- a/src/audit/mod.rs
+++ b/src/audit/mod.rs
@@ -13,6 +13,7 @@ pub(crate) mod dangerous_triggers;
 pub(crate) mod excessive_permissions;
 pub(crate) mod hardcoded_container_credentials;
 pub(crate) mod impostor_commit;
+pub(crate) mod insecure_commands;
 pub(crate) mod known_vulnerable_actions;
 pub(crate) mod ref_confusion;
 pub(crate) mod self_hosted_runner;
diff --git a/src/main.rs b/src/main.rs
index f861f41..f2ea26e 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -154,6 +154,7 @@ fn run() -> Result<ExitCode> {
     register_audit!(audit::self_hosted_runner::SelfHostedRunner);
     register_audit!(audit::known_vulnerable_actions::KnownVulnerableActions);
     register_audit!(audit::unpinned_uses::UnpinnedUses);
+    register_audit!(audit::insecure_commands::InsecureCommands);
 
     let bar = ProgressBar::new((workflow_registry.len() * audit_registry.len()) as u64);
 


Test patch:
diff --git a/tests/acceptance.rs b/tests/acceptance.rs
index 23d6c63..dba00b5 100644
--- a/tests/acceptance.rs
+++ b/tests/acceptance.rs
@@ -200,3 +200,25 @@ fn audit_unpinned_uses() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn audit_unsecure_commands_allowed() -> anyhow::Result<()> {
+    let auditable = workflow_under_test("insecure-commands.yml");
+
+    let cli_args = [&auditable];
+
+    let execution = zizmor().args(cli_args).output()?;
+
+    assert_eq!(execution.status.code(), Some(14));
+
+    let findings = serde_json::from_slice(&execution.stdout)?;
+
+    assert_value_match(&findings, "$[0].determinations.confidence", "High");
+    assert_value_match(
+        &findings,
+        "$[0].locations[0].concrete.feature",
+        "ACTIONS_ALLOW_UNSECURE_COMMANDS",
+    );
+
+    Ok(())
+}
diff --git a/tests/test-data/insecure-commands.yml b/tests/test-data/insecure-commands.yml
new file mode 100644
index 0000000..7d43e6e
--- /dev/null
+++ b/tests/test-data/insecure-commands.yml
@@ -0,0 +1,11 @@
+on: pull_request
+
+name: insecure-commands
+
+jobs:
+  some-dangerous-job:
+    runs-on: ubuntu-latest
+    env:
+      ACTIONS_ALLOW_UNSECURE_COMMANDS: yes
+    steps:
+      - run: echo "don't do this"