Skip to content

Commit 15fd62e

Browse files
chaliyclaude
chaliy
and
claude
authored
fix(interpreter): add depth limit to extglob pattern matching (#447)
## Summary - Add MAX_GLOB_DEPTH=50 constant to prevent unbounded recursion - Add depth parameter to glob_match_impl() and match_extglob() - Update all 23 recursive call sites to propagate depth - Returns false (no match) when depth exceeded ## Test plan - [x] 1 new test: extglob +(a|aa) against "aaaaaaaaaaaa" completes in <5s - [x] All 1432 existing tests pass - [x] clippy clean Closes #409 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 48fde7a commit 15fd62e

File tree

1 file changed

+73
-24
lines changed
  • crates/bashkit/src/interpreter

1 file changed

+73
-24
lines changed

‎crates/bashkit/src/interpreter/mod.rs‎

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,8 @@ pub struct Interpreter {
295295
}
296296

297297
impl Interpreter {
298+
const MAX_GLOB_DEPTH: usize = 50;
299+
298300
/// Create a new interpreter with the given filesystem.
299301
pub fn new(fs: Arc<dyn FileSystem>) -> Self {
300302
Self::with_config(fs, None, None, HashMap::new())
@@ -2849,7 +2851,7 @@ impl Interpreter {
28492851

28502852
/// Simple glob pattern matching with support for *, ?, and [...]
28512853
fn glob_match(&self, value: &str, pattern: &str) -> bool {
2852-
self.glob_match_impl(value, pattern, false)
2854+
self.glob_match_impl(value, pattern, false, 0)
28532855
}
28542856

28552857
/// Parse an extglob pattern-list from pattern string starting after '('.
@@ -2902,7 +2904,12 @@ impl Interpreter {
29022904
}
29032905

29042906
/// Glob match with optional case-insensitive mode
2905-
fn glob_match_impl(&self, value: &str, pattern: &str, nocase: bool) -> bool {
2907+
fn glob_match_impl(&self, value: &str, pattern: &str, nocase: bool, depth: usize) -> bool {
2908+
// THREAT[TM-DOS-031]: Bail on excessive recursion depth
2909+
if depth >= Self::MAX_GLOB_DEPTH {
2910+
return false;
2911+
}
2912+
29062913
let extglob = self.is_extglob();
29072914

29082915
// Check for extglob at the start of pattern
@@ -2911,7 +2918,7 @@ impl Interpreter {
29112918
if matches!(bytes[0], b'@' | b'?' | b'*' | b'+' | b'!') && bytes[1] == b'(' {
29122919
let op = bytes[0];
29132920
if let Some((alts, rest)) = Self::parse_extglob_pattern_list(&pattern[2..]) {
2914-
return self.match_extglob(op, &alts, &rest, value, nocase);
2921+
return self.match_extglob(op, &alts, &rest, value, nocase, depth + 1);
29152922
}
29162923
}
29172924
}
@@ -2931,7 +2938,12 @@ impl Interpreter {
29312938
// Extglob *(pattern-list) — collect remaining pattern
29322939
let remaining_pattern: String = pattern_chars.collect();
29332940
let remaining_value: String = value_chars.collect();
2934-
return self.glob_match_impl(&remaining_value, &remaining_pattern, nocase);
2941+
return self.glob_match_impl(
2942+
&remaining_value,
2943+
&remaining_pattern,
2944+
nocase,
2945+
depth + 1,
2946+
);
29352947
}
29362948
pattern_chars.next();
29372949
// * matches zero or more characters
@@ -2942,14 +2954,19 @@ impl Interpreter {
29422954
while value_chars.peek().is_some() {
29432955
let remaining_value: String = value_chars.clone().collect();
29442956
let remaining_pattern: String = pattern_chars.clone().collect();
2945-
if self.glob_match_impl(&remaining_value, &remaining_pattern, nocase) {
2957+
if self.glob_match_impl(
2958+
&remaining_value,
2959+
&remaining_pattern,
2960+
nocase,
2961+
depth + 1,
2962+
) {
29462963
return true;
29472964
}
29482965
value_chars.next();
29492966
}
29502967
// Also try with empty match
29512968
let remaining_pattern: String = pattern_chars.collect();
2952-
return self.glob_match_impl("", &remaining_pattern, nocase);
2969+
return self.glob_match_impl("", &remaining_pattern, nocase, depth + 1);
29532970
}
29542971
(Some('?'), _) => {
29552972
// Check for extglob ?(...)
@@ -2958,7 +2975,12 @@ impl Interpreter {
29582975
if extglob && pc_clone.peek() == Some(&'(') {
29592976
let remaining_pattern: String = pattern_chars.collect();
29602977
let remaining_value: String = value_chars.collect();
2961-
return self.glob_match_impl(&remaining_value, &remaining_pattern, nocase);
2978+
return self.glob_match_impl(
2979+
&remaining_value,
2980+
&remaining_pattern,
2981+
nocase,
2982+
depth + 1,
2983+
);
29622984
}
29632985
if value_chars.peek().is_some() {
29642986
pattern_chars.next();
@@ -2996,6 +3018,7 @@ impl Interpreter {
29963018
&remaining_value,
29973019
&remaining_pattern,
29983020
nocase,
3021+
depth + 1,
29993022
);
30003023
}
30013024
}
@@ -3027,13 +3050,19 @@ impl Interpreter {
30273050
rest: &str,
30283051
value: &str,
30293052
nocase: bool,
3053+
depth: usize,
30303054
) -> bool {
3055+
// THREAT[TM-DOS-031]: Bail on excessive recursion depth
3056+
if depth >= Self::MAX_GLOB_DEPTH {
3057+
return false;
3058+
}
3059+
30313060
match op {
30323061
b'@' => {
30333062
// @(a|b) — exactly one of the alternatives
30343063
for alt in alts {
30353064
let full = format!("{}{}", alt, rest);
3036-
if self.glob_match_impl(value, &full, nocase) {
3065+
if self.glob_match_impl(value, &full, nocase, depth + 1) {
30373066
return true;
30383067
}
30393068
}
@@ -3042,13 +3071,13 @@ impl Interpreter {
30423071
b'?' => {
30433072
// ?(a|b) — zero or one of the alternatives
30443073
// Try zero: skip the extglob entirely
3045-
if self.glob_match_impl(value, rest, nocase) {
3074+
if self.glob_match_impl(value, rest, nocase, depth + 1) {
30463075
return true;
30473076
}
30483077
// Try one
30493078
for alt in alts {
30503079
let full = format!("{}{}", alt, rest);
3051-
if self.glob_match_impl(value, &full, nocase) {
3080+
if self.glob_match_impl(value, &full, nocase, depth + 1) {
30523081
return true;
30533082
}
30543083
}
@@ -3058,19 +3087,19 @@ impl Interpreter {
30583087
// +(a|b) — one or more of the alternatives
30593088
for alt in alts {
30603089
let full = format!("{}{}", alt, rest);
3061-
if self.glob_match_impl(value, &full, nocase) {
3090+
if self.glob_match_impl(value, &full, nocase, depth + 1) {
30623091
return true;
30633092
}
30643093
// Try alt followed by more +(a|b)rest
30653094
// We need to try consuming `alt` prefix then matching +(...)rest again
30663095
for split in 1..=value.len() {
30673096
let prefix = &value[..split];
30683097
let suffix = &value[split..];
3069-
if self.glob_match_impl(prefix, alt, nocase) {
3098+
if self.glob_match_impl(prefix, alt, nocase, depth + 1) {
30703099
// Rebuild the extglob for the suffix
30713100
let inner = alts.join("|");
30723101
let re_pattern = format!("+({}){}", inner, rest);
3073-
if self.glob_match_impl(suffix, &re_pattern, nocase) {
3102+
if self.glob_match_impl(suffix, &re_pattern, nocase, depth + 1) {
30743103
return true;
30753104
}
30763105
}
@@ -3081,22 +3110,22 @@ impl Interpreter {
30813110
b'*' => {
30823111
// *(a|b) — zero or more of the alternatives
30833112
// Try zero
3084-
if self.glob_match_impl(value, rest, nocase) {
3113+
if self.glob_match_impl(value, rest, nocase, depth + 1) {
30853114
return true;
30863115
}
30873116
// Try one or more (same as +(...))
30883117
for alt in alts {
30893118
let full = format!("{}{}", alt, rest);
3090-
if self.glob_match_impl(value, &full, nocase) {
3119+
if self.glob_match_impl(value, &full, nocase, depth + 1) {
30913120
return true;
30923121
}
30933122
for split in 1..=value.len() {
30943123
let prefix = &value[..split];
30953124
let suffix = &value[split..];
3096-
if self.glob_match_impl(prefix, alt, nocase) {
3125+
if self.glob_match_impl(prefix, alt, nocase, depth + 1) {
30973126
let inner = alts.join("|");
30983127
let re_pattern = format!("*({}){}", inner, rest);
3099-
if self.glob_match_impl(suffix, &re_pattern, nocase) {
3128+
if self.glob_match_impl(suffix, &re_pattern, nocase, depth + 1) {
31003129
return true;
31013130
}
31023131
}
@@ -3110,17 +3139,20 @@ impl Interpreter {
31103139
// Actually: !(pat) matches anything that doesn't match @(pat)
31113140
let inner = alts.join("|");
31123141
let positive = format!("@({}){}", inner, rest);
3113-
!self.glob_match_impl(value, &positive, nocase)
3114-
&& self.glob_match_impl(value, rest, nocase)
3142+
!self.glob_match_impl(value, &positive, nocase, depth + 1)
3143+
&& self.glob_match_impl(value, rest, nocase, depth + 1)
31153144
|| {
31163145
// !(pat) can also consume characters — try each split
31173146
for split in 1..=value.len() {
31183147
let prefix = &value[..split];
31193148
let suffix = &value[split..];
31203149
// prefix must not match any alt
3121-
let prefix_matches_any =
3122-
alts.iter().any(|a| self.glob_match_impl(prefix, a, nocase));
3123-
if !prefix_matches_any && self.glob_match_impl(suffix, rest, nocase) {
3150+
let prefix_matches_any = alts
3151+
.iter()
3152+
.any(|a| self.glob_match_impl(prefix, a, nocase, depth + 1));
3153+
if !prefix_matches_any
3154+
&& self.glob_match_impl(suffix, rest, nocase, depth + 1)
3155+
{
31243156
return true;
31253157
}
31263158
}
@@ -8171,7 +8203,7 @@ impl Interpreter {
81718203
continue;
81728204
}
81738205

8174-
if self.glob_match_impl(&entry.name, &file_pattern, nocase) {
8206+
if self.glob_match_impl(&entry.name, &file_pattern, nocase, 0) {
81758207
// Construct the full path
81768208
let full_path = if path.is_absolute() {
81778209
dir.join(&entry.name).to_string_lossy().to_string()
@@ -8255,7 +8287,7 @@ impl Interpreter {
82558287
if entry.name.starts_with('.') && !dotglob && !pattern_starts_with_dot {
82568288
continue;
82578289
}
8258-
if self.glob_match_impl(&entry.name, pat, nocase) {
8290+
if self.glob_match_impl(&entry.name, pat, nocase, 0) {
82598291
matches.push(dir.join(&entry.name).to_string_lossy().to_string());
82608292
}
82618293
}
@@ -9483,4 +9515,21 @@ mod tests {
94839515
let result = run_script("_READONLY_myvar=1; myvar=hello; echo $myvar").await;
94849516
assert_eq!(result.stdout.trim(), "hello");
94859517
}
9518+
9519+
#[tokio::test]
9520+
async fn test_extglob_no_hang() {
9521+
use std::time::{Duration, Instant};
9522+
let start = Instant::now();
9523+
let result = run_script(
9524+
r#"shopt -s extglob; [[ "aaaaaaaaaaaa" == +(a|aa) ]] && echo yes || echo no"#,
9525+
)
9526+
.await;
9527+
let elapsed = start.elapsed();
9528+
assert!(
9529+
elapsed < Duration::from_secs(5),
9530+
"extglob took too long: {:?}",
9531+
elapsed
9532+
);
9533+
assert_eq!(result.exit_code, 0);
9534+
}
94869535
}

0 commit comments

Comments
 (0)