search

sile / libflate

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

Private function prefix() is unsound #29

Closed Shnatsel closed 5 years ago

Shnatsel commented 5 years ago

The following function may perform out-of-bounds reads if used incorrectly:

https://github.com/sile/libflate/blob/14c762714d937db8c788f10540f27e69d6f0fc5a/src/lz77/default.rs#L112-L120

There have already been two known cases of out-of-bounds reads due to misuse of this function: #16, #21.

In the current implementation it's the caller's responsibility to ensure no out-of-bounds reads occur. If left as-is, this function must be marked unsafe. A better option would be getting rid of unsafety entirely.

Shnatsel commented 5 years ago

I feel memory safety is really critical for libflate for two reasons:

  1. It's used in the enormously popular reqwest crate, where it's subjected to untrusted data from the network. Assuming the limitation of reading only 2 bytes out of bounds at a time can be bypassed by supplying inputs of varying sizes, this issue skirts dangerously close to heartbleed. Actually no, libflate is a development dependency of reqwest.
  2. If I were okay with sacrificing memory safety for performance, I'd just use the popular C libraries. Memory safety should be the differentiating factor of a Rust implementation.