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

parse_headers() enables undefined behavior #34

Closed
abonander opened this issue Mar 5, 2017 · 5 comments
Closed

parse_headers() enables undefined behavior #34

abonander opened this issue Mar 5, 2017 · 5 comments

Comments

@abonander
Copy link
Contributor

abonander commented Mar 5, 2017

Simplest reproduction.

extern crate httparse;

use httparse::{EMPTY_HEADER, parse_headers};

fn main() {
    let mut buf = *b"Foo: Bar\r\n\r\n";

    let mut headers = [EMPTY_HEADER];

    let headers_len = {
        let (_, headers) = parse_headers(&mut buf, &mut headers).unwrap().unwrap();
        headers.len()
    } ;

    assert_eq!(headers_len, 1);

    buf[0] = b'B';

    // Prints "Boo"
    println!("{:?}", headers[0].name);
}

As you can see, parse_headers() allows borrows to buf to escape in headers, creating a double-borrow where the original buffer can be mutated while views to it exist.

Discovered by accident, I was working on some infinite-loop bugs in multipart when I took a double-take at this function and thought, "Wait a minute, how the hell did this work to begin with?" The r.consume() at 80 shouldn't be allowed, but the borrow is escaping.

abonander added a commit to abonander/multipart that referenced this issue Mar 5, 2017
abonander added a commit to abonander/multipart that referenced this issue Mar 5, 2017
@seanmonstar
Copy link
Owner

I believe this is caused by the implementation of the internal shrink function, which is just trying to shorten the reference it has to the headers slice. In my experimenting, it doesn't seem to be immediately obvious how to do so and make the borrow checker happy...

@abonander
Copy link
Contributor Author

Holy crap, this may be Rust's failing unless I'm fundamentally misunderstanding something here: https://is.gd/pFdpPJ

@abonander
Copy link
Contributor Author

So yeah, it's a bug with Rust, haha.

@seanmonstar
Copy link
Owner

@abonander Whew! I was genuinely staring at this last night, and wondering "but, I have lifetimes specified, how did I screw this up?!"

@abonander
Copy link
Contributor Author

rust-lang/rust#40288 is closed now and any breakage was addressed via Crater run so I think this is safe to close now.

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

2 participants