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

Untrusted input can be crafted to cause large internal allocations #11

Closed
udoprog opened this issue May 18, 2023 · 1 comment
Closed

Comments

@udoprog
Copy link

udoprog commented May 18, 2023

Found through fuzzing, should be reproducible if you check out musli and run this:

cargo +nightly miri run --bin fuzz --no-default-features --features model_dlhn,dlhn -- --random --iter 10
MIRI backtrace
error: resource exhaustion: tried to allocate more memory than available to compiler
   --> C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\alloc.rs:164:14
    |
164 |     unsafe { __rust_alloc_zeroed(layout.size(), layout.align()) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ tried to allocate more memory than available to compiler
    |
    = note: inside `std::alloc::alloc_zeroed` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\alloc.rs:164:14: 164:64
    = note: inside `std::alloc::Global::alloc_impl` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\alloc.rs:175:43: 175:63
    = note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate_zeroed` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\alloc.rs:240:9: 240:38
    = note: inside `alloc::raw_vec::RawVec::<u8>::allocate_in` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\raw_vec.rs:185:38: 185:67      
    = note: inside `alloc::raw_vec::RawVec::<u8>::with_capacity_zeroed_in` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\raw_vec.rs:138:9: 138:62
    = note: inside `<u8 as std::vec::spec_from_elem::SpecFromElem>::from_elem::<std::alloc::Global>` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\vec\spec_from_elem.rs:52:31: 52:72
    = note: inside `std::vec::from_elem::<u8>` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\vec\mod.rs:2550:5: 2550:52
    = note: inside `dlhn::de::Deserializer::<'_, &[u8]>::new_dynamic_buf` at C:\Users\udoprog\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\macros.rs:47:36: 47:69
    = note: inside `<&mut dlhn::de::Deserializer<'_, &[u8]> as serde::de::Deserializer<'_>>::deserialize_string::<serde::de::impls::StringVisitor>` at C:\Users\udoprog\.cargo\registry\src\index.crates.io-6f17d22bba15001f\dlhn-0.1.5\src\de.rs:217:28: 217:50
    = note: inside `serde::de::impls::<impl serde::de::Deserialize<'_> for std::string::String>::deserialize::<&mut dlhn::de::Deserializer<'_, &[u8]>>` at C:\Users\udoprog\.cargo\registry\src\index.crates.io-6f17d22bba15001f\serde-1.0.163\src\de\impls.rs:586:9: 586:55
    = note: inside `<std::marker::PhantomData<std::string::String> as serde::de::DeserializeSeed<'_>>::deserialize::<&mut dlhn::de::Deserializer<'_, &[u8]>>` at C:\Users\udoprog\.cargo\registry\src\index.crates.io-6f17d22bba15001f\serde-1.0.163\src\de\mod.rs:791:9: 791:37
    = note: inside `<dlhn::de::StructDeserializer<'_, '_, &[u8]> as serde::de::MapAccess<'_>>::next_value_seed::<std::marker::PhantomData<std::string::String>>` at C:\Users\udoprog\.cargo\registry\src\index.crates.io-6f17d22bba15001f\dlhn-0.1.5\src\de.rs:457:9: 457:50
    = note: inside `<dlhn::de::StructDeserializer<'_, '_, &[u8]> as serde::de::MapAccess<'_>>::next_value::<std::string::String>` at C:\Users\udoprog\.cargo\registry\src\index.crates.io-6f17d22bba15001f\serde-1.0.163\src\de\mod.rs:1870:9: 1870:42
note: inside `<musli_tests::models::_::<impl serde::de::Deserialize<'de> for musli_tests::models::Allocated>::deserialize::__Visitor<'_> as serde::de::Visitor<'_>>::visit_map::<dlhn::de::StructDeserializer<'_, '_, &[u8]>>`
   --> D:\Repo\projects\repos\musli\crates\musli-tests\src\models.rs:106:49

This is the problematic line: https://github.com/otake84/dlhn/blob/6f25c178a255c93eab6f18aa3ca5e4b11b504380/dlhn/src/de.rs#LL58C12-L58C12.

The cause is fairly straight forward: a vector is being eagerly allocated and zerod, its size is picked verbatim from the byte stream. So a few bytes worth of payload can therefore be used to allocate as much memory as you want to on the target machine without it actually being filled with any input data.

There are several ways to mitigate this:

  • Apply a limit to how much you pre-allocate when allocating data structures. They will then asymptotically grow as they're being filled with input. This is the most common approach, and this way in order to actually cause huge allocations, a huge amount of input data has to be processed as well which usually is already limited.
  • You can apply a limit to how much you'll allocate in total. But that's hard with rust allocation APIs being unstable right now. I will certainly do this when allocator apis are stable.
@otake84
Copy link
Owner

otake84 commented May 18, 2023

Your comment was quite surprising to me.
I've addressed this issue by limiting the allocation of Vec to a maximum of 128.
This fix has been released in version 0.1.6.
I sincerely thank you for your detailed report and the method of reproduction.

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