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

DoS risk: panic "index out of bounds" while building very small regex #464

Closed
PaulGrandperrin opened this issue Apr 14, 2018 · 4 comments
Closed
Labels

Comments

@PaulGrandperrin
Copy link

Hi,

regex::Regex::new("a{\r\n");

will cause

thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1'

playground

I found it while porting https://github.com/rust-fuzz/targets to afl.rs and honggfuzz (it's currently only using libFuzzer).
It's funny because libFuzzer seems unable to find it while honggfuzz finds it reliably in just a couple of seconds and AFL in a couple of dozen of minutes.

Regexes sometimes are built from untrusted input so I guess it could be used for denial of service.

@robertswiecki : I found it with honggfuzz first, is that trophy worthy?

@BurntSushi
Copy link
Member

I was stumped for a moment because I couldn't reproduce it with the following program:

extern crate regex;

use regex::Regex;

fn main() {
    let re = Regex::new(r"a{\r\n");
    println!("{:?}", re);
}

Running gives a syntax error, not a panic, as expected:

$ ./target/debug/regex-464
Err(Syntax(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
regex parse error:
    a{\r\n
      ^
error: decimal literal empty
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
))

But it seems the issue here is that I used a raw string. If I use "a{\r\n" as in your example, then I can indeed reproduce this problem. The panic is actually coming from the error formatter, not the parser, which is interesting!

However you found this, it's definitely a legitimate bug, and I would consider it trophy worthy. :-)

@BurntSushi
Copy link
Member

BurntSushi commented Apr 14, 2018

A fix should now be on crates.io in regex-syntax 0.5.5.

@PaulGrandperrin
Copy link
Author

Awesome, thanks @BurntSushi !

@robertswiecki
Copy link

@PaulGrandperrin nice!! here's the trophy update - google/honggfuzz@ef1aa31#diff-04c6e90faac2675aa89e2176d2eec7d8

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

No branches or pull requests

3 participants