integer overflow on a particular regex

[finnbear]

What version of regex are you using?

1.8.1 (latest)

Describe the bug at a high level.

The following code causes an integer-overflow panic in debug mode:

fn main() {
    regex::Regex::new("  {2147483516}{2147483416}{5}").unwrap();
}

What are the steps to reproduce the behavior?

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0f0b53f5c300a71ce6098e767d13cbc2

What is the actual behavior?

• integer overflow
  • panic in debug mode
  • seemingly returns the right answer (Err(CompiledTooBig(10485760))) anyway in release mode

What is the expected behavior?

Return the right answer (Err(CompiledTooBig(10485760))) without integer overflow.

Acknowledgement

The fuzzing infrastructure that found this bug was provided to and integrated into surrealdb by Google OSS Fuzz and @silvergasp, respectively.

The consensus from reporting this as a security issue was that it shouldn't be treated as one as users can avoid panics using catch_unwind. The security policy and/or documentation may be updated to reflect this.

[nathaniel-brough]

Hmm it might be worth adding --debug-assertions to the regex fuzzer in OS's fuzz here https://github.com/google/oss-fuzz/blob/master/projects/rust-regex/build.sh#L18

I would guess that's why it shows up in the surrealdb fuzzer but not the regex fuzzer.

[BurntSushi]

This is fixed in regex 1.8.2 (and regex-syntax 0.7.2) on crates.io.

@silvergasp For some reason I thought the fuzzer infrastructure enabled debug assertions automatically somehow. I'll investigate that.

[nathaniel-brough]

Yeah the -O flag is equivalent to compiling in release mode (which is what you want for fuzzing, faster is better). To add debug assertions you'll need cargo fuzz build -O --debug-assertions. There is a note about it in the oss-fuzz documentation

[BurntSushi]

Yeah, sorry, what I meant is that in the course of fuzzing I had thought I saw panics that were only produced by debug_assertions being enabled. But my memory is fallible.

Would it be better to add debug-assertions = true to the Cargo.toml instead of changing the OSS-fuzz invocation? The former is under my control. The latter isn't.

[nathaniel-brough]

Up to you really, another alternative would be to open a PR with oss-fuzz. For projects that need to be able to easily edit the build.sh file on a regular basis I've actually moved that file from oss-fuzz->upstream_project. You just need to copy it in from the git repo in the Dockerfile. Happy to open a couple of PR's (1 here and 1 on oss-fuzz) to do that if you like?

[nathaniel-brough]

^^ this would also make it pretty trivial to add new fuzzers to oss-fuzz on a regular basis.

[BurntSushi]

Yeah I need to add new fuzzers to OSS-fuzz for the regex 1.9 release. PRs would be great, thank you!