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

Evaluate flate2 #143

Closed
RazrFalcon opened this issue Jun 20, 2019 · 6 comments
Closed

Evaluate flate2 #143

RazrFalcon opened this issue Jun 20, 2019 · 6 comments
Milestone

Comments

@RazrFalcon
Copy link
Collaborator

Looks like flate2 is more safe than libflate.

@RazrFalcon RazrFalcon added this to the v0.8.0 milestone Jun 20, 2019
@Shnatsel
Copy link
Contributor

Shnatsel commented Jun 20, 2019

I'm now working my way through libflate, reporting sometimes and fixing the issues I discover:
sile/libflate#29
sile/libflate#31
sile/libflate#33
So it is improving.

I couldn't get any bugs out of miniz_oxide (the Rust backend for flate2) with a fuzzer, including when fuzzing with libdiffuzz. I have to admit that reasoning about the unsafety in miniz_oxide is beyond my abilities, but it doesn't look very reassuring. Issues like Frommi/miniz_oxide#14 also don't instill confidence.

I can vouch for the safety of https://github.com/image-rs/inflate because I've purged all but one unsafe blocks in it, and then found and fixed the bug in the remaining unsafe block together with the maintainer. But last time I checked it was even slower than libflate.

@Shnatsel
Copy link
Contributor

Shnatsel commented Jul 4, 2019

I'm done cleansing libflate of unsafe blocks. Now it only has one, which I've verified to be sound. Another one has been punted into a separate crate, where the implementation is heavily based on a stdlib function. So I can vouch for the safety of libflate as well, starting with version 0.1.25.

Also, I've since discovered a heap buffer overflow in miniz_oxide and likely also miniz. There is still a lot of unsafe code in them that I was unable to audit, so I would advise against using them if safety is a concern.

@RazrFalcon
Copy link
Collaborator Author

Nice! Than I will force the minimum libflate version.

@Shnatsel
Copy link
Contributor

I believe this issue should be revisited. The Rust backend for flate2 called miniz_oxide is now 100% safe and forbids unsafe code. It is also 2.5x faster on decompression than libflate.

There is still some unsafety in flate2, so I'd suggest using miniz_oxide directly. This would improve performance, provide compiler-guaranteed safety and reduce the number of dependencies.

@RazrFalcon
Copy link
Collaborator Author

Thanks. I will try it out.

@RazrFalcon
Copy link
Collaborator Author

I've switched to flate2, but not to miniz_oxide directly. Since I need gzip support.

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