Patching this properly will require not just replacing a centralized copy of libpng, but scanning all binaries for statically-linked copies. There might be some tools left over from the last time libpng got into trouble; this would be a very good time to update and link to them. Some categories of software that are at particularly high risk right now:
* Web browsers. Sandboxing likely helps, but there's a possibility that there are some usages (eg favicons) that decode outside the sandbox.
* MMS on Android (possibly remote root on phones with no user interaction)
* Any server that accepts PNG uploads and processes them (eg, user avatar thumbnail generation)
* Anything which shows people user-provided avatar images (eg, IM clients)
* Video games which download levels that can contain images
* File browsers which show image previews
* Music players which display album covers
Expect malicious images to start appearing everywhere, and soon. If you're responsible for the computer security anywhere, this is a drop-everything priority.
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/unnoticed.
So stuff like this happens [this is a tongue in cheek scenario, treat it as joke, not criticism of particular people or framework]:
Developer1: "Argh, I can't figure out dependencies for this library",
Then someone comes along and "fixes" it:
Developer2: "Look, just static compile it / include all deps in a subdir some place. See, fixed! I even made a language / framework thingie to do this for you. Yeah, those stupid old packagers and OS people made such a mess...".
Developer1: "Oh wow so easy, I'll be using this and tell others to use it too." /goes to twitter...
A few years later, everyone uses the new shiny dependency-bundling library/framework thingie. Then a security vulnerabily hits and everyone realizes why those old boring packagers and OS people set things up the way they did. 6 days later everyone forgets and jump on the next new shiny framework. Cycle repeats...
So you've rediscovered what UNIX has allowed us to do for decades now.
Things like process isolation, memory isolation, file isolation, and hardware isolation aren't new. UNIX systems offered mature implementations of them in the 1980s, if not before then.
Best of all, this functionality was offered without needing to resort to resource-intensive virtual machines or anything of that sort.
Those who want the level of isolation you propose have been able to get it for a long time now. They just tend not to, because it's inconvenient and impractical for real world systems.
The only exception would be something like Chrome/Chromium that bundles a bunch of its own tweaked copies of common libraries. Or various closed-source software not controlled by your package manager, like some commercial games which have a linux build available. But many systems, like my home router/fileserver, and my workstation, don't have any of these exceptions, and they were updated in about 2 minutes. (I have no long running network daemons using libpng, but if I did they would be easy to restart.)
Which is why we should have stopped using C long ago and used languages which could guarantee that kind of isolation with less overhead.
Anyway, this is exactly the question I wanted to ask, whether stuff like this counts against bundling. I am, however, not very experienced in these things, I'm curious if anyone else who has more experience with distribution of software could weigh in on whether there is a way to mitigate this issue when it comes to people who choose to bundle software for ease for users' sake.
For the developer, interfaces are cleaner, but for users, the interface style is leaky where PIMPL is not. That makes it superior. It also avoids overriding the intent of inheritance as a way of achieving ABI compatibility when it already has a significant and intentional function of runtime polymorphism, and can easily confuse users as to whether the implementation is to preserve the ABI or provide multiple implementations in the same release.
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.
I'm also a Gentoo user. (Hi! :D )
ISTR that preserved-libs "just" lets you delay recompilation of affected packages  by keeping the old libs around until everything that needs rebuilding to link against the new version has been rebuilt. You still need to rebuild packages. Am I wrong about that?
 Or, -alternatively- "just" lets you keep your system working while you rebuild affected packages.
Oh god, tell me about it. I was around for the first libpng upgrade catastrophe, way back when. Portage/emerge has gotten so much better since then. :D
With zlib I agree though. Static linking doesn't make sense with such a standard library, at least under unix systems.
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.
For example a GUI that loads icons, background images, button images etc.
Kinda good example right here: https://www.youtube.com/watch?v=CkPAgv1Gjz0
Maybe Nintendo never considered someone screwing around with their network to load an arbitrary image in a place that only Nintendo-Generated Mii png images should be but then I found a way... and if that libpng was vulnerable, suddenly we've gained usermode execution on a Wii U. Or worse yet, similar to the Sony PSP .tiff image exploit, maybe after gaining usermode we find a kernel-mode exploit and attack that. Now, we're near the realm of game piracy and private keys.
> allow remote attackers to cause a denial of service (application crash) or possibly have unspecified other impact
As far as I understand, trying to predict what is possible by exploiting a buffer overflow is hard because it depends on many specifics, such as compiler options used, OS protections, ...
Crashes are better than privilege escalation, but they do not mean that privilege escalation is impossible.
For instance the JVM uses a Java PNG decoder. So it isn't vulnerable.
However, I don't know if piston-image specifically provides such a C interface.
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.
size_t sz = lib_get_buffer_size();
char *buf = malloc(sz);
It's a deficiency in the API, which should require the buffer size to be passed to lib_fill_buffer so lib_fill_buffer doesn't have to make any assumptions about the size. But if you're trying to preserve compatibility with existing C APIs, you might be stuck with APIs like this.
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 language? Hopefully the likes of Rust bring and end to this.
Perhaps there's a culture of C++ being the only proper language for libraries?
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.
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).
Some applications might read the bit depth from the IHDR chunk and allocate
memory for a 2^N entry palette, while libpng can return a palette with up
to 256 entries even when the bit depth is less than 8.
Unfortunately RFC 2083 contains the requirement that a compliant PLTE will not be larger than the bit depth. So I'm sure many applications that trusted libpng to do the validation will be exploitable. I consider this a caution against promising too much in the spec, as well as not overly relying on an implementation to exactly follow the spec. Remember the robustness principle: be conservative in what you send, be liberal in what you accept.
If a PNG app trusts the spec, and the library doesn't comply, and that causes the app to crash. That's a good thing overall. It would lead to somebody discovering the problem and hopefully fixing it. It's only a bad thing for the specific app that crashes. Or in this case, risks getting exploited. Maybe for security, the principle doesn't apply the same way.
I'd wager that there is one libpng and then only about 4 or 5 distinct pieces of code that use it that everyone else in turn is using. If that code is wrong, that would still make it widely vulnerable.
If everyone is using common glue code then I think it's more likely this won't be a problem since the middleware library probably noticed the bug and works around it. It's when developers hastily reinvent the wheel that sloppy validation bugs like this are problems. It's why the maxim "never write your own crypto" exists. Or as Mark Twain said, "put all your eggs in one basket and _watch_that_basket_."
But as I said, there's no actual buffer overflow in libpng. This is a validation bug. The headline and the NVD summary are misleading.
Some other image libraries are hard because they force you to write many code-paths to handle various different cases, but libpng provides many different helpers that make it pretty simple.
The PNG format has lots of variations and optional bits that would be quite annoying if you needed to handle them all yourself. I'm referring to the many various flags, options, and little helper functions in libpng that make handling all these format variations quite painless for the user of libpng.
[Some libraries, e.g. libtiff, provide mechanisms to abstract out the low-level details, but still expect the application to explicitly handle many format variations, and this is quite annoying.]
libpng is hardly perfect; it's old, and that certainly shows in various ways (e.g. the use of setjmp). But to call it the "worst API ever" is just wrong.
> Some other image libraries are hard because they force you to write many code-paths to handle various different cases, but libpng provides many different helpers that make it pretty simple.
It seemed that you were thinking of error code handling as a "code-path" and setjmp/longjmp being a mechanism to reduce code paths.
Anyway, it sounds like we're in agreement. It's not that big of a deal to use libpng directly, and "worst API ever" is too strong. The author has probably not done enough Windows API programming to find the worst API ever :-).
>and 1.6.x before 1.6.19
Unfortunately, the latest version on libpng site  is 1.6.18.
Why was this CVE announced before the patch and version update was released?
> UPDATE 12 November 2015:
> The latest released versions are libpng-1.5.24 and libpng-1.6.19 [DOWNLOAD].
I don't mean to suggest that this is "really" an application bug or that people shouldn't upgrade libpng ASAP; I'm just wondering whether there are other options for installations/applications where that would be painful for some reason.
Most modern operating systems have buffer overflow protection technologies such as ASLR. I tried recently exploiting a few guaranteed buffer overflows for fun, and it's getting irritatingly hard at least on Linux. Non-executable stack, *alloc functions have sanity checks, -fstack-protector provided canaries, ... It's possible to get past all that, but it takes a bit work.
I would be freaked if I was running some older operating system, and someone vendored a poorly compiled version of libpng. Windows applications are probably the scariest here, especially when run on older Windows servers...
From a quick search in MXR (https://mxr.mozilla.org/mozilla-central/) for the identifiers "png_set_PLTE" and "png_set_PLTE", it seems that at least Mozilla (Firefox) did the lazy thing and always allocated 256 entries.
Edit: I was right, from https://bugzilla.mozilla.org/show_bug.cgi?id=1224244 "Mozilla is not vulnerable to the security issues that it fixes, when using either the in-tree libpng or the system libpng."
The first/latest one ("avoid potential pointer overflow") sounds scary, but I believe it's just about the pathological case where the library is handed (valid) buffers that extend to only a few bytes away from the maximum possible value (i.e. 0xfffff...). If your OS does this, it probably shouldn't.
Going down the list, there are three relevant-looking commits. Listed in chronological order:
[libpng16] Reject attempt to write over-length PLTE chunk https://github.com/glennrp/libpng/commit/81f44665cce4cb1373f...
[libpng16] Prevent reading over-length PLTE chunk (Cosmin Truta). https://github.com/glennrp/libpng/commit/a901eb3ce6087e0afee...
[libpng16] Silently truncate over-length PLTE chunk while reading.
- The first one only substantively changes png_write_PLTE, so it could only be an issue for applications that actually write out PNGs. It changes a check on the ‘num_pal’ argument from a hardcoded 256, aka PNG_MAX_PALETTE_LENGTH, to be based on the bit depth. The png_write_PLTE function is internal and called only from png_write_info, which passes it ‘info_ptr->palette’ and ‘info_ptr->num_palette’ as ‘palette’ and ‘num_pal’, respectively.
- The second one confusingly changes png_write_PLTE again, but just to rename a variable. The substantive change is a similar check on an argument to png_set_PLTE, which is a public function and, incidentally, the only thing that can set ‘info_ptr->num_palette’, unless the application accesses it directly, which is deprecated. (Thus, outside of the deprecated case, it should ensure that the behavior change in the previous patch never actually gets exercised.)
- The third one adds yet another check, to png_handle_PLTE, which is called when reading a PLTE chunk in a PNG file - this time, the variable is called ‘num’. After the check, ‘num’ is used to fill an array of size PNG_MAX_PALETTE_LENGTH - but there was already an earlier check for the length being too much for that, in which case it may longjmp out of the function with png_chunk_error or just return. png_handle_PLTE then calls png_set_PLTE with the same argument, so, in lieu of the added check, the previous commit’s check would still trigger and fail the image load. The new check just changes an error into something the library might be able to recover from.
So the second commit is the important one, and the third demonstrates how png_set_PLTE can be called while reading a PNG with a size argument greater than appropriate for the bit depth, but still <= PNG_MAX_PALETTE_LENGTH. What happens then? The oss-security post says:
> Some applications might read the bit depth from the IHDR
chunk and allocate memory for a 2^N entry palette, while libpng can return
a palette with up to 256 entries even when the bit depth is less than 8.
So are only applications that do something odd affected? The png_get_PLTE function gives the client the actual num_palette value as an out argument, so any client that uses that to size buffers wouldn’t be affected; nor would one that hardcoded 256 or PNG_MAX_PALETTE_LENGTH. For example, Skia does the latter in SkImageDecoder_libpng.cpp.
Are there any libpng-internal uses that could be affected? I grepped for ‘->(num_)?palette\b’. TLDR: I don’t think so, but if you want the pointless detail, among other less interesting uses were:
/* Report invalid palette index; added at libng-1.5.10 */
if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE &&
png_ptr->num_palette_max > png_ptr->num_palette)
png_image_read_header uses it to set image->colormap_entries, which has its own long list of uses. I think this is then used (by the application, via PNG_IMAGE_COLORMAP_SIZE) to determine the size of the colormap buffer, but there may be an issue somewhere.
png_image_read_colormap uses it to generate actual color map data, but this is not obviously dependent on the bit depth, and checked against image->colormap_entries (…in case it changed from earlier?).
png_set_quantize uses a user-specified num_palette value and in any case is not important functionality.
Some PNG_COMPOSE code uses num_palette to write to palette; not a problem…
png_do_expand_palette just looks up entries in palette (png_ptr->palette) by byte-sized indices without checking anything. The palette buffer must be at least 256 entries long, or else it will read out of bounds, which is presumably why ->palette is only set here:
/* Changed in libpng-1.2.1 to allocate PNG_MAX_PALETTE_LENGTH instead
* of num_palette entries, in case of an invalid PNG file or incorrect
* call to png_set_PLTE() with too-large sample values.
png_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr,
PNG_MAX_PALETTE_LENGTH * (sizeof (png_color))));
png_handle_tRNS and png_handle_hIST have separate arrays whose count is expected to be <= and ==, respectively, num_palette.
- For trans, the ‘trans_alpha’ buffer itself is always PNG_MAX_PALETTE_LENGTH sized, so there can be no overflow there; it also sets info_ptr->num_trans. This can affect the application if it uses png_get_tRNS.
- The hist buffer is also always maximum size and doesn’t have a separate length field, so it’s covered by me checking uses of num_palette.
png_do_check_palette_indexes checks the bit-depth invariant separately.
hIST again in pngwrite.c and pngwutil.c, nothing else interesting in those files.
I think num_trans is safe in the same way, but I haven’t looked as closely.
I have web applications that probably use libpng five different ways, each from a different client application and libpng version.
Knowing that image libraries may contain vulnerabilities, how would you design a secure image ingestion process?
Some reasonable strategies...
(0) Attack surface minimization: Similarly to other types of input, input is segregated and treated as toxic and dangerous until proven otherwise. (Eg. Don't store random inbound files where you store those you will re-use; never allow batch processes to touch temporary files received from external systems; delete external input as soon as possible; store input only after normalization)
(1) Attack surface minimization: only accept one file format. This provides no guarantees but helps to limit exposure to unknown vulnerabilities, though may not always be feasible.
(2) Parallelization (opposite of the above): explicitly convert all formats of input to some common format (using throwaway containers/VMs), ideally using multiple versions of multiple libraries on multiple operating systems, and compare the results. If the results of every environment do not match, raise an alarm and do not continue processing.
(3) Reverse-conversion check. Convert the file to another (non-lossy conversion possible) format, then convert it back (also in a throwaway environment). If the result matches (or the portion of the result that is non-lossy matches) continue. This could be combined with #2 for additional strength.
(4) "Application-level firewall" approach: progressively determine the various format-specific and general properties of the image in question (eg. starting with libmagic; and preferably all in relatively secured throwaway environments, using the above strategies!) then apply a neural network or basic statistical analysis to generate alerts for anomiles. For instance, the reduced pallette featured in this attack would not be normal for many types of image input (scans or photographs come to mind). Other useful checks may include "are the image dimensions sane?", "is the aspect ratio sane?", "is the image predominantly black and white?", etc.
Strategies #0 (in particular cases), #1 (with some luck), #2, #3 and #4 (with some luck) could have mitigated this attack. Executed wrongly, any of those strategies could also have triggered the vulnerability. #4 is ripe for a web2.0 Silly-Valley outsource-the-problem startup (investors?).
PS. I enjoy these types of problems; anyone looking for ideas or development get in touch.
What fraction of applications that read png images use libpng?
How many of those applications have features that load 3rd party images (uploadable avatar etc)?
My gut feeling is that the most common usage is for loading things like UI elements, and not for external images? Are such programs safe from exploitation?