Skip to content

Commit 0388c46

Browse files
chaliyclaude
andauthored
fix(interpreter): detect cyclic nameref to prevent wrong resolution (#465)
## Summary - Cyclic namerefs (a->b->a) silently resolved to a wrong variable - Added HashSet-based visited tracking in `resolve_nameref()` to detect cycles - Returns the original name when a cycle is detected ## Test plan - [x] Unit test: `test_cyclic_nameref_detected` Closes #426 Co-authored-by: Claude <noreply@anthropic.com>
1 parent e40a5f6 commit 0388c46

File tree

1 file changed

+21
-2
lines changed
  • crates/bashkit/src/interpreter

1 file changed

+21
-2
lines changed

crates/bashkit/src/interpreter/mod.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7639,11 +7639,16 @@ impl Interpreter {
76397639
/// follow the chain (up to 10 levels to prevent infinite loops).
76407640
fn resolve_nameref<'a>(&'a self, name: &'a str) -> &'a str {
76417641
let mut current = name;
7642+
let mut visited = std::collections::HashSet::new();
7643+
visited.insert(name);
76427644
for _ in 0..10 {
76437645
let key = format!("_NAMEREF_{}", current);
76447646
if let Some(target) = self.variables.get(&key) {
7645-
// target is owned by the HashMap, so we can return a reference to it
7646-
// But we need to work with &str. Let's use a different approach.
7647+
// THREAT[TM-INJ-011]: Detect cyclic namerefs and stop.
7648+
if !visited.insert(target.as_str()) {
7649+
// Cycle detected — return original name (Bash emits a warning)
7650+
return name;
7651+
}
76477652
current = target.as_str();
76487653
} else {
76497654
break;
@@ -9542,4 +9547,18 @@ mod tests {
95429547
// Should be sandboxed value (1), not real PID
95439548
assert_eq!(pid, 1, "$$ should return sandboxed PID, not real host PID");
95449549
}
9550+
9551+
// Issue #426: cyclic nameref should not resolve to wrong variable
9552+
#[tokio::test]
9553+
async fn test_cyclic_nameref_detected() {
9554+
let mut bash = crate::Bash::new();
9555+
// Create cycle: a -> b -> a
9556+
let result = bash
9557+
.exec("declare -n a=b; declare -n b=a; a=hello; echo $a")
9558+
.await
9559+
.unwrap();
9560+
// With the bug, this would silently resolve to an arbitrary variable.
9561+
// With the fix, the cycle is detected and 'a' resolves to itself.
9562+
assert_eq!(result.exit_code, 0);
9563+
}
95459564
}

0 commit comments

Comments
 (0)