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

Unchecked vector pre-allocation #151

Open
dbrgn opened this issue Nov 21, 2017 · 8 comments

Comments

@dbrgn
Copy link

commented Nov 21, 2017

When playing around with afl, I found the following example:

extern crate rmpv;

use std::io::Cursor;

fn main() {
    let data = [219, 175, 142, 142, 201, 219, 128, 0, 50, 175, 142, 196, 100, 212, 185];
    let mut cursor = Cursor::new(data);
    let decoded = rmpv::decode::value::read_value(&mut cursor);
    println!("Done: {:?}", decoded);
}

It takes almost 200ms in release mode and 2min 47s in debug mode to run.

@dbrgn

This comment has been minimized.

Copy link
Author

commented Nov 21, 2017

Note that – at least in debug mode – the CPU usage is at 100% on one core and memory consumption is steadily increasing to a few gigabytes.

@dbrgn

This comment has been minimized.

Copy link
Author

commented Nov 21, 2017

Sorry for the triple post. If I understand the msgpack spec correctly, this is because the data indicates that a raw str buffer with 2.8 GiB of data is following.

The decoder should probably abort after having reached the end of the input data, right? Or is this an issue with the Cursor?

@3Hren

This comment has been minimized.

Copy link
Owner

commented Nov 21, 2017

Yep, there is vector pre-allocation to fit the specified size (which is 2945355465 bytes) before actual reading. This is definitely a bug.

@dbrgn dbrgn changed the title Slow decoding Unchecked vector pre-allocation Mar 8, 2018

@stusmall

This comment has been minimized.

Copy link

commented Mar 6, 2019

I took a quick look at fixing this. Looks like it should be easy to fix once rust-lang/rust#48043 stabilizes

@dbrgn

This comment has been minimized.

Copy link
Author

commented Mar 6, 2019

The question is whether we should even try to allocate that much memory, or whether a heuristic should be used to check whether there will actually be data in the input buffer to fill that memory...

E.g. if the remaining input data (in a non-streaming parser) is smaller than the requested buffer size, then something's probably wrong.

@huitseeker

This comment has been minimized.

Copy link

commented Aug 27, 2019

This issue came up in the recent audit of the dalek libraries by Quarkslab.

Is it worth opening a RustSec report?

@dbrgn

This comment has been minimized.

Copy link
Author

commented Aug 27, 2019

Let's ask @tarcieri 🙂 It does have potential for denial-of-service.

@tarcieri

This comment has been minimized.

Copy link

commented Aug 28, 2019

This particular vulnerability has come up enough times in my life I'd appreciate an advisory for it 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.