Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

CVE-2015-8126: Multiple buffer overflows in libpng

web.nvd.nist.gov

Post reply on HN
Print view View original

81–88 of 88 posts

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#81

Post by kibwen »

agwa wrote: post #79
kibwen wrote: post #61

Earlier quoted context omitted.

I see there are some downvotes here, which I don't think are deserved. It's true that at some level of your stack you're going to have some code that's just poking bytes into buffers, and that generally takes some manual effort to verify. For what it's worth, I'd expect this sort of thing to be walled up behind an `unsafe` block in Rust, which if nothing else would lend increased scrutiny in an audit.

I believe the point tedunangst is trying to make is that if the API works something like this: size_t sz = lib_get_buffer_size(); char *buf = malloc(sz); lib_fill_buffer(buf); and there's a bug in lib_get_buffer_size that causes it to return too small a number, but lib_fill_buffer assumes it's big enough, you've got a vulnerability regardless of what language the library is written in. This isn't a bug in the applica…

To clarify further, the idiomatic usage of `unsafe` in Rust stipulates that if you can't guarantee that your function is memory-safe for all possible inputs, then you must mark the function itself as `unsafe` to force callers to be aware of the risk. Obviously if you're both calling this theoretical function from a language without an `unsafe` construct and if you're also striving to maintain exact API compatibility with the C function then you can't really make this aware to the caller. If you do have control of the API then the way that this would generally be presented on the Rust side would be to have two functions: a safe one named "foo" that also takes the length as an argument so that you can check at runtime and an unsafe function named "unsafe_foo" that has the same behavior as the C function.

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#82

Post by technion »

Asbostos wrote: post #74
killercup wrote: post #11

A good opportunity to check how https://github.com/PistonDevelopers/image-png is doing (a PNG decoder written in Rust). Looks like it includes bindings to use miniz.c for DEFLATE decoding as well as "inflate" (which seems to be DEFLATE in Rust). Also, it seems to have a fuzzing driver (png-afl). Good times!

Why on earth isn't this common practice for any software that can be remotely controlled over the internet? Ie anything rendering parts of web pages, opening untrusted files, etc? Everyone has known what a massive security risk unsafe languages are for a long time. Nearly every vulnerability is a buffer overflow. What's the value in persisting in writing such dangerous code? Just because it's 10% faster than a safer…

Libpng isn't exactly new. libpng.org seems to be offline at the moment so I'm not having a lot of success finding details - but it's safe bet that it predates Rust, afl-fuzz, and probably even clang's static analyser.

I'm all for saying "now is the time to rewrite ", but it's hard to get upset at that project when anyone else could have gone and done a modern take on the project - I hadn't heard of the earlier quoted Rust PNG library until this happened.

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#83

Post by cesarb »

technion wrote: post #82
Asbostos wrote: post #74

Earlier quoted context omitted.

Why on earth isn't this common practice for any software that can be remotely controlled over the internet? Ie anything rendering parts of web pages, opening untrusted files, etc? Everyone has known what a massive security risk unsafe languages are for a long time. Nearly every vulnerability is a buffer overflow. What's the value in persisting in writing such dangerous code? Just because it's 10% faster than a safer…

Libpng isn't exactly new. libpng.org seems to be offline at the moment so I'm not having a lot of success finding details - but it's safe bet that it predates Rust, afl-fuzz, and probably even clang's static analyser. I'm all for saying "now is the time to rewrite ", but it's hard to get upset at that project when anyone else could have gone and done a modern take on the project - I hadn't heard of the earlier quoted…

> but it's safe bet that it predates Rust, afl-fuzz, and probably even clang's static analyser.

Not only it predates clang's static analyser, it predates clang itself! The site is now online, and I can see on it news about libpng from last century (1995 to be more exact).

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#84

Post by librarian42 »

throwaway2048 wrote: post #71
zanny wrote: post #62

Earlier quoted context omitted.

Then don't use those libraries! How can you trust a third party library to provide functionality to your software if they cannot even release sanely with ABI stability? Its a disservice to you and your users to risk security vulnerabilities like this.

with tons of functionality there is no realistic alternative.

Simplify the interface. Don't combine unrelated functionality in a single library.

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#85

Post by librarian42 »

Asbostos wrote: post #76
throwaway2048 wrote: post #21

Earlier quoted context omitted.

This is exactly why static linking/bundled libs/containerization is such a piss poor idea. We are going to be dealing with this vuln for years and years, if not decades to come. The exact same insanity has has happened with embedded copies of zlib in the past. The solution to dependency management in way too many cases becomes "never update". And those mechanisms allow negligence like that to fester for years ignored…

Static linking may be bad for security but good for saving hours of frustration. It's a trade-off and it's not clear that security is automatically the most important thing always. Just look at any forum about some open source software. There'll usually be endless posts from people complaining that they can't compile it because of some weird dependency errors. These are really widespread and time-wasting problems.

Unnecessary dependencies are a bug. I've seen people pull in half a dozen nested libraries just to avoid writing ten lines of code. Sometimes re-inventing the wheel is the right thing to do.

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#86

Post by rewqfdsa »

zanny wrote: post #59
rewqfdsa wrote: post #41

Earlier quoted context omitted.

PIMPL is an inefficient antipattern when the holder object is itself heap-allocated. You're better off coding against interfaces, COM-style.

It doesn't matter if your holder is heap or stack allocated, the d pointer is always heap allocated and the only reference to it is the private internal. With interfaces you either have an awful dummy static constructor for your private object somewhere or you have a non-virtual parent class with dummy implementations of all its functions and a mangled constructor that produces its private child. For the developer, i…

Right, so if your holder object is heap-allocated, you need to dereference a pointer so that you can then call into the implementation. That's inefficient, bad for the cache, and puts needless pressure on the heap allocator. C++ classes built out of pure virtual member functions are just as compatible. If you use something COM-like, you can reference-count and manage them in a uniform way.

PIMPL as commonly described is a waste of good electricity, like most design patterns. Especially because most of the time I see it used, it's from the keyboard of some well-meaning but inexperienced developer who doesn't even need to maintain binary compatibility.

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#87

Post by fanf2 »

marssaxman wrote: post #57
throwaway2048 wrote: post #21

Earlier quoted context omitted.

This is exactly why static linking/bundled libs/containerization is such a piss poor idea. We are going to be dealing with this vuln for years and years, if not decades to come. The exact same insanity has has happened with embedded copies of zlib in the past. The solution to dependency management in way too many cases becomes "never update". And those mechanisms allow negligence like that to fester for years ignored…

Containerization might well be the solution to this kind of problem: libpng's vulnerabilities no longer matter much if you've isolated it into its own VM with no access to resources beyond the ones it needs to decode a PNG into a pixel buffer. I've been working on a little OS project along these lines, where every process runs in its own VM, and every shared service is provided by a local server. There's no dynamic l…

Sounds like Qubes

Re: CVE-2015-8126: Multiple buffer overflows in libpng

#88

Post by PinkysBrain »

marssaxman wrote: post #57
throwaway2048 wrote: post #21

Earlier quoted context omitted.

This is exactly why static linking/bundled libs/containerization is such a piss poor idea. We are going to be dealing with this vuln for years and years, if not decades to come. The exact same insanity has has happened with embedded copies of zlib in the past. The solution to dependency management in way too many cases becomes "never update". And those mechanisms allow negligence like that to fester for years ignored…

Containerization might well be the solution to this kind of problem: libpng's vulnerabilities no longer matter much if you've isolated it into its own VM with no access to resources beyond the ones it needs to decode a PNG into a pixel buffer. I've been working on a little OS project along these lines, where every process runs in its own VM, and every shared service is provided by a local server. There's no dynamic l…

We have no processors which can efficiently isolate memory spaces with that fine granularity.

Which is why we should have stopped using C long ago and used languages which could guarantee that kind of isolation with less overhead.

Post reply on HN