search

rust-secure-code / safety-dance

Auditing crates for unsafe code which can be safely replaced
Apache License 2.0
536 stars 10 forks source link

Audit gif #24

Closed Shnatsel closed 5 years ago

Shnatsel commented 5 years ago

https://crates.io/crates/gif

Pure-Rust GIF decoder, used in image and everything that relies on it. 2000 downloads/day. High risk due to parsing untrusted data in a binary format.

Shnatsel commented 5 years ago

The C API is where most of the unsafety lies. It is currently highly experimental and can be ignored. However, it looks like the core library could be made 100% safe.

I've started purging unsafe code, but I will not be able to finish the job. Done so far: https://github.com/image-rs/image-gif/pull/61 https://github.com/image-rs/image-gif/pull/63

Shnatsel commented 5 years ago

My work is merged, so there are only two unsafe blocks remaining outside the C API, both doing the same thing - transmuting the lifetime away: https://github.com/image-rs/image-gif/blob/2dcbe4f82e296e3eed2d4a408e71557eefe46176/src/reader/decoder.rs#L217 https://github.com/image-rs/image-gif/blob/2dcbe4f82e296e3eed2d4a408e71557eefe46176/src/reader/mod.rs#L114

Shnatsel commented 5 years ago

The 2 remaining unsafe blocks actually pass Polonius checks, see https://github.com/danielhenrymantilla/image-gif/tree/polonius-fix

So I'm considering them audited and assume that 100% safety is blocked until Polonius.

Shnatsel commented 5 years ago

Tracking issue on rustc side: https://github.com/rust-lang/rust/issues/51545