Skip to content

Commit 0c90959

Browse files
authored
fix(rg): bound --colors parsing and sanitize invalid spec echo (#1782)
### Motivation - The new `--colors` parser previously collected all colon-separated fields into a `Vec`, allowing an attacker-controlled spec with many delimiters to amplify allocations and trigger memory pressure or OOM. - Error diagnostics echoed the full untrusted spec, risking large diagnostic allocations and information amplification. ### Description - Replace `spec.split(':').collect()` with bounded parsing using `splitn(4, ':')` and validate fields incrementally to avoid allocation amplification. - Add `MAX_COLOR_SPEC_LEN` to reject overly long `--colors` inputs early. - Introduce `invalid_color_spec_error` to truncate and sanitize echoed specs in error messages (limits echoed chars with `...`). - Add two focused unit tests: `colors_rejects_too_long_spec_with_truncated_echo` and `colors_rejects_extra_delimiters_without_split_collect_amplification` to cover rejection and diagnostic truncation. ### Testing - Ran the focused tests with `cargo test -p bashkit colors_rejects`, and both new tests passed. - The package test run that executed these tests reported `2 passed; 0 failed` for the new cases and completed the crate test run successfully (other tests were filtered as usual). ------ [Codex Task](https://chatgpt.com/codex/cloud/tasks/task_e_6a13b309e0dc832baa49b1e8cc9b0c8d)
1 parent ed919a7 commit 0c90959

1 file changed

Lines changed: 53 additions & 18 deletions

File tree

  • crates/bashkit/src/builtins/rg

crates/bashkit/src/builtins/rg/mod.rs

Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -196,21 +196,29 @@ impl Default for RgColorScheme {
196196

197197
impl RgColorScheme {
198198
fn apply(&mut self, spec: &str) -> Result<()> {
199-
let parts: Vec<&str> = spec.split(':').collect();
200-
if parts.len() == 2 && parts[1] == "none" {
201-
self.style_mut(parts[0])?.disable();
199+
const MAX_COLOR_SPEC_LEN: usize = 256;
200+
if spec.len() > MAX_COLOR_SPEC_LEN {
201+
return Err(invalid_color_spec_error(spec));
202+
}
203+
204+
let mut fields = spec.splitn(4, ':');
205+
let field0 = fields.next().unwrap_or_default();
206+
let field1 = fields.next();
207+
let field2 = fields.next();
208+
let extra = fields.next();
209+
210+
if field1 == Some("none") && field2.is_none() && extra.is_none() {
211+
self.style_mut(field0)?.disable();
202212
return Ok(());
203213
}
204-
if parts.len() != 3 {
205-
return Err(Error::Execution(format!(
206-
"rg: error parsing flag --colors: invalid color spec '{spec}'"
207-
)));
214+
if field1.is_none() || field2.is_none() || extra.is_some() {
215+
return Err(invalid_color_spec_error(spec));
208216
}
209-
let style = self.style_mut(parts[0])?;
210-
match parts[1] {
211-
"fg" => style.set_fg(parse_ansi_fg(parts[2])?),
212-
"bg" => style.set_bg(parse_ansi_bg(parts[2])?),
213-
"style" => match parts[2] {
217+
let style = self.style_mut(field0)?;
218+
match field1.unwrap_or_default() {
219+
"fg" => style.set_fg(parse_ansi_fg(field2.unwrap_or_default())?),
220+
"bg" => style.set_bg(parse_ansi_bg(field2.unwrap_or_default())?),
221+
"style" => match field2.unwrap_or_default() {
214222
"bold" => style.set_bold(true),
215223
"nobold" => style.set_bold(false),
216224
"intense" => style.set_intense(true),
@@ -222,15 +230,11 @@ impl RgColorScheme {
222230
_ => {
223231
return Err(Error::Execution(format!(
224232
"rg: error parsing flag --colors: invalid style '{}'",
225-
parts[2]
233+
field2.unwrap_or_default()
226234
)));
227235
}
228236
},
229-
_ => {
230-
return Err(Error::Execution(format!(
231-
"rg: error parsing flag --colors: invalid color spec '{spec}'"
232-
)));
233-
}
237+
_ => return Err(invalid_color_spec_error(spec)),
234238
}
235239
Ok(())
236240
}
@@ -249,6 +253,17 @@ impl RgColorScheme {
249253
}
250254
}
251255

256+
fn invalid_color_spec_error(spec: &str) -> Error {
257+
const MAX_SPEC_ECHO_CHARS: usize = 80;
258+
let mut truncated = spec.chars().take(MAX_SPEC_ECHO_CHARS).collect::<String>();
259+
if spec.chars().count() > MAX_SPEC_ECHO_CHARS {
260+
truncated.push_str("...");
261+
}
262+
Error::Execution(format!(
263+
"rg: error parsing flag --colors: invalid color spec '{truncated}'"
264+
))
265+
}
266+
252267
impl RgColorStyle {
253268
fn plain() -> Self {
254269
Self {
@@ -6093,6 +6108,26 @@ mod tests {
60936108
assert!(err.to_string().contains("ignore file too large"));
60946109
}
60956110

6111+
#[test]
6112+
fn colors_rejects_too_long_spec_with_truncated_echo() {
6113+
let mut scheme = RgColorScheme::default();
6114+
let spec = "x".repeat(300);
6115+
let err = scheme.apply(&spec).expect_err("spec should be rejected");
6116+
let msg = err.to_string();
6117+
assert!(msg.contains("invalid color spec"));
6118+
assert!(msg.contains("..."));
6119+
assert!(!msg.contains(&spec));
6120+
}
6121+
6122+
#[test]
6123+
fn colors_rejects_extra_delimiters_without_split_collect_amplification() {
6124+
let mut scheme = RgColorScheme::default();
6125+
let err = scheme
6126+
.apply("match:fg:blue:extra")
6127+
.expect_err("spec with extra delimiter should be rejected");
6128+
assert!(err.to_string().contains("invalid color spec"));
6129+
}
6130+
60966131
async fn run_rg(args: &[&str], stdin: Option<&str>, files: &[(&str, &[u8])]) -> ExecResult {
60976132
run_rg_with_cwd(args, stdin, files, "/").await
60986133
}

0 commit comments

Comments
 (0)