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

Panic when decompressing #2

Closed
frewsxcv opened this issue Oct 25, 2015 · 6 comments
Closed

Panic when decompressing #2

frewsxcv opened this issue Oct 25, 2015 · 6 comments
Assignees
Labels

Comments

@frewsxcv
Copy link
Contributor

extern crate brotli;

use std::io::{self, Read};
use brotli::Decompressor;

fn main() {
    let mut input = vec![];
    let _ = Decompressor::new(&b"\x1b\x3f\xff\xff\xdb\x4f\xe2\x99\x80\x12".to_vec() as &[u8]).read_to_end(&mut input);
}

Crash discovered using afl.rs

@ende76
Copy link
Owner

ende76 commented Oct 25, 2015

@frewsxcv Thanks! I will look at it right now.

@ende76
Copy link
Owner

ende76 commented Oct 25, 2015

@frewsxcv I have identified the issue, and must admit that I had already been aware of it, yet postponed solving it. The answer was actually in one of the last few remaining @todo annotations:

    // @TODO: probably need to use parameter alphabet_size here to be able to
    //        reject streams with excessive repeated trailing zeros, as per section
    //        3.5. of the RFC:
    //        "If the number of times to repeat the previous length
    //         or repeat a zero length would result in more lengths in
    //         total than the number of symbols in the alphabet, then the
    //         stream should be rejected as invalid."

Your stream managed to create a seemingly endless stream of zeros for the runlength encoding, which seemed to have created a very long or possibly even endless loop. I have pushed a fix, but will keep this issue open for a little longer, to analyze your stream manually, because I want to make sure that's what happened.

@frewsxcv
Copy link
Contributor Author

Should a regression test be added?

@ende76
Copy link
Owner

ende76 commented Oct 25, 2015

Yes, absolutely! At the moment, I'm still analyzing your input in detail.

@frewsxcv
Copy link
Contributor Author

Alright, let me know when you think this is solved and I can continue fuzzing

@ende76
Copy link
Owner

ende76 commented Oct 25, 2015

Sorry, this took much longer than I first anticipated. The symptom was fixed 2 hours ago, and that fix should have been in place anyway, as already mentioned.
However, the analysis of your stream has brought to my attention a somewhat more serious problem, where I was a bit too cavalier with possible IO errors when parsing runlength codes (the exact place where your stream sent me looping).
Getting that right required many small changes all over, but I thought it was worth it to get it really right.
So now the issue can be considered closed.

@ende76 ende76 closed this as completed Oct 25, 2015
@ende76 ende76 added the bug label Oct 25, 2015
@ende76 ende76 self-assigned this Oct 25, 2015
@frewsxcv frewsxcv changed the title Crash when decompressing Panic when decompressing Oct 25, 2015
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

2 participants