sile / libflate

A Rust implementation of DEFLATE algorithm and related formats (ZLIB, GZIP)
https://docs.rs/libflate
MIT License
178 stars 35 forks source link

Decoder::read_non_compressed_block() is unsound #31

Closed Shnatsel closed 5 years ago

Shnatsel commented 5 years ago

The following code is unsound:

https://github.com/sile/libflate/blob/0f565d36d10d5b163b5c3c208c162dd5b8c8c87a/src/deflate/decode.rs#L86-L91

The slice passed to read_exact() is uninitialized. This uses a Read implementation supplied by the API user, and there is no guarantee that it will never read from the provided buffer. If it does, it may cause a memory disclosure vulnerability.

Similar bug in Rust MP4 parser for reference: https://github.com/mozilla/mp4parse-rust/issues/172

The equivalent code in stdlib initializes the vector with zeroes before growing it: https://doc.rust-lang.org/src/std/io/mod.rs.html#355-391

There have been some language proposals to create a contract for never reading from the buffer in this case, but they have not been stabilized: https://github.com/rust-lang/rust/issues/42788

For now replacing unsafe { self.buffer.set_len(old_len + len as usize) }; with self.buffer.resize(old_len + len as usize, 0); should fix it.

I have not read all of the unsafe code in libflate, there may be similar issues in other unsafe blocks, which is why I'm opening an issue instead of a PR right away.

Shnatsel commented 5 years ago

This code might also be affected: https://github.com/sile/libflate/blob/0f565d36d10d5b163b5c3c208c162dd5b8c8c87a/src/gzip.rs#L1122

sile commented 5 years ago

Thank you for your detailed report. I would like to fix unsound code one by one.

Shnatsel commented 5 years ago

This seems to be the only occurrence of this issue. Everywhere else read_exact() is passed properly initialized buffers.

Shnatsel commented 5 years ago

Fixed by the linked PR.