Skip to content

Commit bc0e914

Browse files
feat: unique parser instance for all audit executions
1 parent da740d7 commit bc0e914

File tree

1 file changed

+59
-47
lines changed

1 file changed

+59
-47
lines changed

Diff for: src/audit/github_env.rs

+59-47
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,25 @@ use crate::models::Step;
44
use crate::state::AuditState;
55
use anyhow::Context;
66
use github_actions_models::workflow::job::StepBody;
7+
use std::cell::RefCell;
78
use std::ops::Deref;
89
use tree_sitter::Parser;
910

10-
pub(crate) struct GitHubEnv;
11+
pub(crate) struct GitHubEnv {
12+
// Note : interior mutability adopted since Parser::parse requires &mut self
13+
bash_script_paster: RefCell<Parser>,
14+
}
1115

1216
audit_meta!(GitHubEnv, "github-env", "dangerous use of GITHUB_ENV");
1317

1418
impl GitHubEnv {
15-
fn evaluate_github_environment_within_bash_script(script_body: &str) -> anyhow::Result<bool> {
16-
let bash = tree_sitter_bash::LANGUAGE;
17-
let mut parser = Parser::new();
18-
parser
19-
.set_language(&bash.into())
20-
.context("failed to load bash parser")?;
21-
let tree = parser
19+
fn evaluate_github_environment_within_bash_script(
20+
&self,
21+
script_body: &str,
22+
) -> anyhow::Result<bool> {
23+
let tree = &self
24+
.bash_script_paster
25+
.borrow_mut()
2226
.parse(script_body, None)
2327
.context("failed to parse bash script body")?;
2428

@@ -32,7 +36,7 @@ impl GitHubEnv {
3236
tree_expansion.contains(">>") || tree_expansion.contains(">");
3337

3438
// Eventually we can detect specific commands within the expansion,
35-
// tee and others
39+
// like tee and others
3640
let piped = tree_expansion.contains("|");
3741

3842
if (piped || exploitable_redirects) && targets_github_env {
@@ -48,12 +52,12 @@ impl GitHubEnv {
4852
Ok(false)
4953
}
5054

51-
fn uses_github_environment(run_step_body: &str, shell: &str) -> anyhow::Result<bool> {
55+
fn uses_github_environment(&self, run_step_body: &str, shell: &str) -> anyhow::Result<bool> {
5256
// Note : as an upcoming refinement, we should evaluate shell interpreters
5357
// other than Bash
5458

5559
match shell {
56-
"bash" => Self::evaluate_github_environment_within_bash_script(run_step_body),
60+
"bash" => self.evaluate_github_environment_within_bash_script(run_step_body),
5761
&_ => {
5862
log::warn!(
5963
"'{}' shell not supported when evaluating usage of GITHUB_ENV",
@@ -70,7 +74,14 @@ impl WorkflowAudit for GitHubEnv {
7074
where
7175
Self: Sized,
7276
{
73-
Ok(Self)
77+
let bash = tree_sitter_bash::LANGUAGE;
78+
let mut parser = Parser::new();
79+
parser
80+
.set_language(&bash.into())
81+
.context("failed to load bash parser")?;
82+
Ok(Self {
83+
bash_script_paster: RefCell::new(parser),
84+
})
7485
}
7586

7687
fn audit_step<'w>(&self, step: &Step<'w>) -> anyhow::Result<Vec<Finding<'w>>> {
@@ -87,7 +98,7 @@ impl WorkflowAudit for GitHubEnv {
8798

8899
if let StepBody::Run { run, shell, .. } = &step.deref().body {
89100
let interpreter = shell.clone().unwrap_or("bash".into());
90-
if Self::uses_github_environment(run, &interpreter)? {
101+
if self.uses_github_environment(run, &interpreter)? {
91102
findings.push(
92103
Self::finding()
93104
.severity(Severity::High)
@@ -109,50 +120,51 @@ impl WorkflowAudit for GitHubEnv {
109120
#[cfg(test)]
110121
mod tests {
111122
use crate::audit::github_env::GitHubEnv;
123+
use crate::audit::WorkflowAudit;
124+
use crate::state::{AuditState, Caches};
112125

113126
#[test]
114127
fn test_exploitable_bash_patterns() {
115-
for case in &[
128+
for (case, expected) in &[
116129
// Common cases
117-
"echo foo >> $GITHUB_ENV",
118-
"echo foo >> \"$GITHUB_ENV\"",
119-
"echo foo >> ${GITHUB_ENV}",
120-
"echo foo >> \"${GITHUB_ENV}\"",
130+
("echo foo >> $GITHUB_ENV", true),
131+
("echo foo >> \"$GITHUB_ENV\"", true),
132+
("echo foo >> ${GITHUB_ENV}", true),
133+
("echo foo >> \"${GITHUB_ENV}\"", true),
121134
// Single > is buggy most of the time, but still exploitable
122-
"echo foo > $GITHUB_ENV",
123-
"echo foo > \"$GITHUB_ENV\"",
124-
"echo foo > ${GITHUB_ENV}",
125-
"echo foo > \"${GITHUB_ENV}\"",
135+
("echo foo > $GITHUB_ENV", true),
136+
("echo foo > \"$GITHUB_ENV\"", true),
137+
("echo foo > ${GITHUB_ENV}", true),
138+
("echo foo > \"${GITHUB_ENV}\"", true),
126139
// No spaces
127-
"echo foo>>$GITHUB_ENV",
128-
"echo foo>>\"$GITHUB_ENV\"",
129-
"echo foo>>${GITHUB_ENV}",
130-
"echo foo>>\"${GITHUB_ENV}\"",
140+
("echo foo>>$GITHUB_ENV", true),
141+
("echo foo>>\"$GITHUB_ENV\"", true),
142+
("echo foo>>${GITHUB_ENV}", true),
143+
("echo foo>>\"${GITHUB_ENV}\"", true),
131144
// tee cases
132-
"something | tee $GITHUB_ENV",
133-
"something | tee \"$GITHUB_ENV\"",
134-
"something | tee ${GITHUB_ENV}",
135-
"something | tee \"${GITHUB_ENV}\"",
136-
"something|tee $GITHUB_ENV",
137-
"something |tee $GITHUB_ENV",
138-
"something| tee $GITHUB_ENV",
145+
("something | tee $GITHUB_ENV", true),
146+
("something | tee \"$GITHUB_ENV\"", true),
147+
("something | tee ${GITHUB_ENV}", true),
148+
("something | tee \"${GITHUB_ENV}\"", true),
149+
("something|tee $GITHUB_ENV", true),
150+
("something |tee $GITHUB_ENV", true),
151+
("something| tee $GITHUB_ENV", true),
152+
("echo foo >> $OTHER_ENV # not $GITHUB_ENV", false),
153+
("something | tee \"${$OTHER_ENV}\" # not $GITHUB_ENV", false),
139154
] {
140-
let uses_github_env = GitHubEnv::uses_github_environment(case, "bash")
141-
.expect("test case is not valid Bash");
142-
assert!(uses_github_env);
143-
}
144-
}
155+
let audit_state = AuditState {
156+
pedantic: false,
157+
offline: false,
158+
gh_token: None,
159+
caches: Caches::new(),
160+
};
145161

146-
#[test]
147-
fn test_additional_bash_patterns() {
148-
for case in &[
149-
// Comments
150-
"echo foo >> $OTHER_ENV # not $GITHUB_ENV",
151-
"something | tee \"${$OTHER_ENV}\" # not $GITHUB_ENV",
152-
] {
153-
let uses_github_env = GitHubEnv::uses_github_environment(case, "bash")
162+
let sut = GitHubEnv::new(audit_state).expect("failed to create audit");
163+
164+
let uses_github_env = sut
165+
.uses_github_environment(case, "bash")
154166
.expect("test case is not valid Bash");
155-
assert!(!uses_github_env);
167+
assert_eq!(uses_github_env, *expected);
156168
}
157169
}
158170
}

0 commit comments

Comments
 (0)