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 length can cause memory allocation error. #1

Closed
AgeManning opened this issue Jun 13, 2019 · 2 comments

Comments

@AgeManning
Copy link

commented Jun 13, 2019

When de-serializing bytes, an arbitrary length of the value can be specified. When building data_buf here:
https://github.com/KizzyCode/asn1_der/blob/master/src/der/value.rs#L17
This can lead to memory crashes for arbitrary large length values. Perhaps an upper bound is required to prevent crashing applications when de serializing arbitrary byte arrays.

This can be reproduced in the following example:

use asn1_der::{Asn1Der, FromDerObject};

#[derive(Asn1Der)]
struct Test {
    inner: String,
}

fn main() {
    let bytes = [
        157, 247, 157, 157, 157, 157, 157, 157, 157, 157, 157, 157, 157, 157, 157, 67, 157, 1, 0,
        0, 0, 157, 157, 157, 157, 157, 157, 157, 157,
    ];
    let _ = Test::deserialize(bytes.iter());
}
@KizzyCode

This comment has been minimized.

Copy link
Owner

commented Jun 13, 2019

Damn, you're right; thank you!

I think the best solution for now would be to avoid any explicit preallocation and push the bytes to the vector (since vector allocates in steps this shouldn't be relevant performance degradation):

// Create buffer and fill it with `len` bytes
let mut data_buf = Vec::new();
for _ in 0..len.into() {
    data_buf.push(*source.next().ok_or(Asn1DerError::LengthMismatch)?);
}
Ok(data_buf.into())

@KizzyCode KizzyCode closed this in dbc7ab1 Jun 13, 2019

KizzyCode added a commit that referenced this issue Jun 13, 2019
@KizzyCode

This comment has been minimized.

Copy link
Owner

commented Jun 13, 2019

So, 0.6.2 is published; old versions have been yanked. Thank you very much for the issue 😊

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