Skip to content

Commit d99f228

Browse files
authored
fix(builtins): block unset of internal variables and readonly marker bypass (#1013)
## Summary - Block `unset _READONLY_*` and other internal marker variables via `is_internal_variable()` check - Defense-in-depth: checks in both interpreter's `execute_unset_builtin` and `Unset` builtin struct ## What & Why `unset _READONLY_FOO` could directly remove the readonly marker, making `FOO` mutable again. This defeats readonly protection, which may be security-critical. Now the interpreter's unset handler checks `is_internal_variable()` before removing any variable, blocking manipulation of all internal markers. ## Tests Added - `unset_readonly_marker_blocked` — verifies `unset _READONLY_X` doesn't defeat readonly - `unset_normal_variable_works` — verifies normal unset still works Closes #1006
1 parent 07b499f commit d99f228

File tree

3 files changed

+72
-1
lines changed

3 files changed

+72
-1
lines changed

crates/bashkit/src/builtins/vars.rs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,37 @@ pub struct Unset;
1313

1414
#[async_trait]
1515
impl Builtin for Unset {
16+
// THREAT[TM-INJ-009]: Block unset of internal variables and readonly variables
1617
async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
18+
let mut stderr = String::new();
19+
let mut exit_code = 0;
1720
for name in ctx.args {
21+
// Block unsetting internal marker variables (_READONLY_, _NAMEREF_, etc.)
22+
if is_internal_variable(name) {
23+
stderr.push_str(&format!(
24+
"bash: unset: {name}: cannot unset: readonly variable\n"
25+
));
26+
exit_code = 1;
27+
continue;
28+
}
29+
// Block unsetting readonly variables
30+
let readonly_key = format!("_READONLY_{name}");
31+
if ctx.variables.contains_key(&readonly_key) {
32+
stderr.push_str(&format!(
33+
"bash: unset: {name}: cannot unset: readonly variable\n"
34+
));
35+
exit_code = 1;
36+
continue;
37+
}
1838
ctx.variables.remove(name);
1939
// Note: env is immutable in our model - environment variables
2040
// are inherited and can't be unset by the shell
2141
}
22-
Ok(ExecResult::ok(String::new()))
42+
Ok(ExecResult {
43+
stderr,
44+
exit_code,
45+
..Default::default()
46+
})
2347
}
2448
}
2549

crates/bashkit/src/interpreter/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4605,6 +4605,10 @@ impl Interpreter {
46054605
self.variables.remove(&format!("_NAMEREF_{}", arg));
46064606
} else {
46074607
let resolved = self.resolve_nameref(arg).to_string();
4608+
// THREAT[TM-INJ-009]: Block unset of internal marker variables
4609+
if is_internal_variable(&resolved) {
4610+
continue;
4611+
}
46084612
// THREAT[TM-INJ-019]: Refuse to unset readonly variables
46094613
if self
46104614
.variables

crates/bashkit/tests/blackbox_security_tests.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,49 @@ mod finding_readonly_bypass {
252252
);
253253
}
254254

255+
/// Issue #1006: unset _READONLY_* marker cannot bypass readonly protection.
256+
#[tokio::test]
257+
async fn unset_readonly_marker_blocked() {
258+
let mut bash = tight_bash();
259+
let result = bash
260+
.exec(
261+
r#"
262+
readonly IMPORTANT=secret
263+
unset _READONLY_IMPORTANT 2>/dev/null
264+
IMPORTANT=hacked 2>/dev/null
265+
echo "IMPORTANT=$IMPORTANT"
266+
"#,
267+
)
268+
.await
269+
.unwrap();
270+
assert!(
271+
result.stdout.contains("IMPORTANT=secret"),
272+
"readonly was bypassed by unsetting _READONLY_ marker, got: {}",
273+
result.stdout
274+
);
275+
}
276+
277+
/// Unset of normal non-readonly variables still works.
278+
#[tokio::test]
279+
async fn unset_normal_variable_works() {
280+
let mut bash = tight_bash();
281+
let result = bash
282+
.exec(
283+
r#"
284+
FOO=hello
285+
unset FOO
286+
echo "FOO=${FOO:-empty}"
287+
"#,
288+
)
289+
.await
290+
.unwrap();
291+
assert!(
292+
result.stdout.contains("FOO=empty"),
293+
"expected FOO to be unset, got: {}",
294+
result.stdout
295+
);
296+
}
297+
255298
/// TM-INJ-020: declare cannot overwrite readonly variables.
256299
#[tokio::test]
257300
async fn declare_cannot_overwrite_readonly() {

0 commit comments

Comments
 (0)