nom icon indicating copy to clipboard operation
nom copied to clipboard

`recognize` can crash with 'attempt to subtract with overflow'

Open mullr opened this issue 2 years ago • 4 comments

If you write a parser that returns a constant string in the first tuple position, and you pass that to recognize, you can crash in a very strange way. The reason is pretty obvious now that I tracked it down: recongize uses 'offset', which relies on the returned string being a subslice of the input.

It would be nice if there was a debug assertion in there, at least, to do a bounds check and provide a more helpful panic message.

use nom::{combinator::recognize, IResult};

fn strange_but_technically_valid_parser<'a>(s: &'a str) -> IResult<&'a str, &'a str> {
    Ok(("", s))
}

fn main() {
    let test_str = "test".to_string();
    let _ = recognize(strange_but_technically_valid_parser)(&test_str);
}

For posterity, this is the output:

thread 'main' panicked at 'attempt to subtract with overflow', /home/mullr/.cargo/registry/src/github.com-1ecc6299db9ec823/nom-7.1.1/src/traits.rs:85:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:143:14
   2: core::panicking::panic
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:48:5
   3: <&str as nom::traits::Offset>::offset
             at /home/mullr/.cargo/registry/src/github.com-1ecc6299db9ec823/nom-7.1.1/src/traits.rs:85:5
   4: nom::combinator::recognize::{{closure}}
             at /home/mullr/.cargo/registry/src/github.com-1ecc6299db9ec823/nom-7.1.1/src/combinator/mod.rs:514:21
   5: nom_recognize_crash_repro::main
             at ./src/main.rs:9:13
   6: core::ops::function::FnOnce::call_once
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

rustc 1.60.0 (7737e0b5c 2022-04-04)
nom 7.1.1, no extra features

mullr avatar Apr 07 '22 22:04 mullr

Yeah, you can't do that. The "rest" you return needs to be a subslice of your input.

Xiretza avatar Apr 08 '22 07:04 Xiretza

You're violating nom invariants so it makes sense that it would panic. Your parser is syntactically safe Rust but not a valid parser.

LoganDark avatar May 12 '22 17:05 LoganDark

Yes, this is clear. I'm suggesting that the invariants could be checked with a debug assert (or perhaps some other mechanism), to save future users some debugging time.

mullr avatar May 12 '22 17:05 mullr

Yes, this is clear. I'm suggesting that the invariants could be checked with a debug assert (or perhaps some other mechanism), to save future users some debugging time.

I'd say it's probably easy enough for anyone to open a PR for that. I can do it for you if you want.

LoganDark avatar May 12 '22 17:05 LoganDark