Skip to content

fix(expansion): keep operand quote state out of variable data - #1948

Merged
chaliy merged 1 commit into
mainfrom
2026-06-05-propose-fix-for-quoted-wildcard-vulnerability
Jun 5, 2026
Merged

chaliy merged 1 commit into
mainfrom
2026-06-05-propose-fix-for-quoted-wildcard-vulnerability

Conversation

@chaliy

@chaliy chaliy commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • Prevent attacker-controlled data from injecting the previous in-band quote marker and disabling quoted-literal protection during parameter-expansion operand parsing.\
  • Preserve bash semantics where quoted portions of pattern operands keep glob metacharacters literal regardless of variable contents.

Description

  • Remove the fixed in-band marker and choose a per-operand quote_mark that is guaranteed absent from the source operand via operand_quote_mark(operand).\
  • Update expand_operand to process parsed literal parts and expanded variable/arithmetic/parameter expansion data separately so quote boundaries are consumed only from parsed literal text and cannot be toggled by expanded data.\
  • Replace strip_operand_quotes(operand) with strip_operand_quotes(operand, quote_mark) and add helpers push_marked_literal, push_operand_expansion, and push_operand_char to escape glob metacharacters in expanded data according to out-of-band quote state.\
  • Remove the global OPERAND_QUOTE_MARK constant and the old escape_marked_glob_literals path.\
  • Add a regression test test_quoted_remove_prefix_operand_marker_byte_keeps_glob_literal that ensures ANSI-C $'\x01' bytes in quoted variable data do not toggle quote protection.

Testing

  • Ran cargo test -p bashkit remove_prefix_operand -- --nocapture and the targeted tests executed successfully (the three related tests passed).\
  • Ran cargo fmt --check and formatting check succeeded.\
  • Ran repository unit tests for affected code paths during development and the new regression tests passed.

Codex Task

Copilot AI review requested due to automatic review settings June 5, 2026 19:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens parameter-expansion operand processing for pattern-removal operators by moving quote-boundary state out of variable/expansion data, preventing attacker-controlled bytes from toggling quoted-literal protection and ensuring quoted glob metacharacters remain literal.

Changes:

  • Replaces the fixed in-band quote marker with a per-operand marker chosen to be absent from the operand source.
  • Refactors expand_operand to consume quote markers only from parsed literal parts and to escape metacharacters in expanded data based on out-of-band quote state.
  • Adds a regression test ensuring $'\x01' in quoted variable data cannot disable quoted glob-literal behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8908 to +8913
fn operand_quote_mark(operand: &str) -> char {
(1..=char::MAX as u32)
.filter_map(char::from_u32)
.find(|&ch| ch != '\0' && !operand.contains(ch))
.expect("Unicode contains at least one char absent from any valid operand")
}
Comment on lines +8931 to 8941
fn push_operand_char(out: &mut String, ch: char, in_marked: bool) {
if in_marked
&& matches!(
ch,
'*' | '?' | '[' | ']' | '(' | ')' | '|' | '+' | '@' | '!'
)
{
out.push('\\');
}
out.push(ch);
}
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 5, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit 91be266 Commit Preview URL Jun 05 2026, 08:08 PM

@chaliy
chaliy merged commit 66ffa6b into main Jun 5, 2026
36 checks passed
@chaliy
chaliy deleted the 2026-06-05-propose-fix-for-quoted-wildcard-vulnerability branch June 5, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants