Skip to content

Commit

Permalink
v0: don't ignore recursion limit failures from any push_depth calls. (
Browse files Browse the repository at this point in the history
  • Loading branch information
eddyb authored Jul 20, 2021
1 parent cce8a07 commit d7ea01a
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 20 deletions.
14 changes: 11 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,17 @@ mod tests {

#[test]
fn limit_recursion() {
use std::fmt::Write;
let mut s = String::new();
assert!(write!(s, "{}", super::demangle("_RNvB_1a")).is_err());
// NOTE(eddyb) the `?` indicate that a parse error was encountered.
// FIXME(eddyb) replace `v0::Invalid` with a proper `v0::ParseError`,
// that could show e.g. `<recursion limit reached>` instead of `?`.
assert_eq!(
super::demangle("_RNvB_1a").to_string().replace("::a", ""),
"?"
);
assert_eq!(
super::demangle("_RMC0RB2_").to_string().replace("&", ""),
"<?>"
);
}

#[test]
Expand Down
68 changes: 51 additions & 17 deletions src/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,13 +568,16 @@ impl<'s> Parser<'s> {

if self.eat(b'B') {
self.backref()?;

self.pop_depth();
return Ok(());
}

let ty_tag = self.next()?;

if ty_tag == b'p' {
// We don't encode the type if the value is a placeholder.
self.pop_depth();
return Ok(());
}

Expand Down Expand Up @@ -653,16 +656,6 @@ impl<'a, 'b, 's> Printer<'a, 'b, 's> {
}
}

fn push_depth(&mut self) -> bool {
match self.parser {
Err(_) => false,
Ok(ref mut parser) => {
let _ = parser.push_depth();
true
}
}
}

fn pop_depth(&mut self) {
if let Ok(ref mut parser) = self.parser {
parser.pop_depth();
Expand Down Expand Up @@ -740,6 +733,8 @@ impl<'a, 'b, 's> Printer<'a, 'b, 's> {
}

fn print_path(&mut self, in_value: bool) -> fmt::Result {
parse!(self, push_depth);

let tag = parse!(self, next);
match tag {
b'C' => {
Expand Down Expand Up @@ -813,14 +808,12 @@ impl<'a, 'b, 's> Printer<'a, 'b, 's> {
self.out.write_str(">")?;
}
b'B' => {
let mut backref_printer = self.backref_printer();
backref_printer.print_path(in_value)?;
if backref_printer.parser.is_err() {
return Err(fmt::Error);
}
self.backref_printer().print_path(in_value)?;
}
_ => invalid!(self),
}

self.pop_depth();
Ok(())
}

Expand All @@ -842,7 +835,7 @@ impl<'a, 'b, 's> Printer<'a, 'b, 's> {
return self.out.write_str(ty);
}

self.push_depth();
parse!(self, push_depth);

match tag {
b'R' | b'Q' => {
Expand Down Expand Up @@ -1009,15 +1002,22 @@ impl<'a, 'b, 's> Printer<'a, 'b, 's> {
}

fn print_const(&mut self) -> fmt::Result {
parse!(self, push_depth);

if self.eat(b'B') {
return self.backref_printer().print_const();
self.backref_printer().print_const()?;

self.pop_depth();
return Ok(());
}

let ty_tag = parse!(self, next);

if ty_tag == b'p' {
// We don't encode the type if the value is a placeholder.
self.out.write_str("_")?;

self.pop_depth();
return Ok(());
}

Expand All @@ -1041,6 +1041,7 @@ impl<'a, 'b, 's> Printer<'a, 'b, 's> {
self.out.write_str(ty)?;
}

self.pop_depth();
Ok(())
}

Expand Down Expand Up @@ -1810,4 +1811,37 @@ RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRB_E"
t_nohash!(&sym, expected);
}
}

#[test]
fn recursion_limit_backref_free_bypass() {
// NOTE(eddyb) this test checks that long symbols cannot bypass the
// recursion limit by not using backrefs, and cause a stack overflow.

// This value was chosen to be high enough that stack overflows were
// observed even with `cargo test --release`.
let depth = 100_000;

// In order to hide the long mangling from the initial "shallow" parse,
// it's nested in an identifier (crate name), preceding its use.
let mut sym = format!("_RIC{}", depth);
let backref_start = sym.len() - 2;
for _ in 0..depth {
sym.push('R');
}

// Write a backref to just after the length of the identifier.
sym.push('B');
sym.push(char::from_digit((backref_start - 1) as u32, 36).unwrap());
sym.push('_');

// Close the `I` at the start.
sym.push('E');

let demangled = format!("{:#}", ::demangle(&sym));

// NOTE(eddyb) the `?` indicates that a parse error was encountered.
// FIXME(eddyb) replace `v0::Invalid` with a proper `v0::ParseError`,
// that could show e.g. `<recursion limit reached>` instead of `?`.
assert_eq!(demangled.replace(&['R', '&'][..], ""), "::<?>");
}
}

0 comments on commit d7ea01a

Please sign in to comment.