Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

recognize can crash with 'attempt to subtract with overflow' #1520

Open
mullr opened this issue Apr 7, 2022 · 4 comments
Open

recognize can crash with 'attempt to subtract with overflow' #1520

mullr opened this issue Apr 7, 2022 · 4 comments

Comments

@mullr
Copy link

mullr commented Apr 7, 2022

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
@Xiretza
Copy link
Contributor

Xiretza commented Apr 8, 2022

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

@LoganDark
Copy link
Contributor

LoganDark commented May 12, 2022

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

@mullr
Copy link
Author

mullr commented May 12, 2022

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.

@LoganDark
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants