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
Closed

Unchecked length can cause memory allocation error. #1

AgeManning opened this issue Jun 13, 2019 · 2 comments

Comments

@AgeManning
Copy link

@AgeManning AgeManning 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
Copy link
Owner

@KizzyCode KizzyCode 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())

Loading

@KizzyCode KizzyCode closed this in dbc7ab1 Jun 13, 2019
KizzyCode added a commit that referenced this issue Jun 13, 2019
@KizzyCode
Copy link
Owner

@KizzyCode KizzyCode commented Jun 13, 2019

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

Loading

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

Successfully merging a pull request may close this issue.

None yet
2 participants