4 ms·

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

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. On initial review, this will require assumptions in the client application to be exploitable. I haven't found a place where libpng itself causes the overrun. It is just an enabler by returning a smaller value for bit_depth than num_palette. If the application double-checks the result from png_get_PLTE or uses PNG_MAX_PALETTE_LENGTH instead of 1<<bit_depth, it won't have a problem except wasting memory. 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.

revelation 11y ago
Does anyone actually interface libpng themselves? The library is pure insanity if all you care about is getting the image out in some format. 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.
- whoopdedo 11y ago
  Github search: png_get_PLTE C 48892 C++ 2916 Obj-C 36 Apple programmers aren't using it directly at least. Nor Android, which is not exploitable[1]. Chromium also uses SKIA I believe. The C number is obviously inflated by the number of repositories that contain a copy of libpng. 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[2], "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. [1]https://android.googlesource.com/platform/external/skia/+/master/src/codec/SkCodec_libpng.cpp#171 https://android.googlesource.com/platform/external/skia/+/ma... [2]maybe
  - Eridrus 11y ago
    Going through some random code on codesearch.com a lot of code seems to just pass in a png_colorp * as the pallete, which this doesn't break.
  - mdaniel 11y ago
    Ah, that's what I came to the comments to find out, having just heard about an Android vulnerability in Chrome. The way the news of that vuln read (that one just needs to visit a web page) I thought this vuln would certainly fit that description.
- asveikau 11y ago
  The libpng simple decoder sample is pretty reasonable C, except for the fact that it uses setjmp to handle errors. (libpng's fault, not the sample.)
- snogglethorpe 11y ago
  libpng is actually very easy to use directly. 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.
  - AndyKelley 11y ago
    Are you referring to setjmp? Because it's quite the opposite. This blog article does a good job explaining: http://latentcontent.net/2007/12/05/libpng-worst-api-ever/ http://latentcontent.net/2007/12/05/libpng-worst-api-ever/
    - snogglethorpe 11y ago
      Huh? I'm not referring to setjmp at all... (why would you think I'm referring to setjmp?) 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.
      - AndyKelley 11y ago
        Ah, I thought you were referring to setjmp with this: > 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 :-).
Asbostos 11y ago
There's now an idea that that robustness principle was backwards. The classic example being web browsers which were so liberal they encouraged all sorts of weird non-standard web pages which now have to be supported forever. 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.

revelation 11y ago

Does anyone actually interface libpng themselves? The library is pure insanity if all you care about is getting the image out in some format. 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.

whoopdedo 11y ago
Github search: png_get_PLTE C 48892 C++ 2916 Obj-C 36 Apple programmers aren't using it directly at least. Nor Android, which is not exploitable[1]. Chromium also uses SKIA I believe. The C number is obviously inflated by the number of repositories that contain a copy of libpng. 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[2], "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. [1]https://android.googlesource.com/platform/external/skia/+/master/src/codec/SkCodec_libpng.cpp#171 https://android.googlesource.com/platform/external/skia/+/ma... [2]maybe
- Eridrus 11y ago
  Going through some random code on codesearch.com a lot of code seems to just pass in a png_colorp * as the pallete, which this doesn't break.
- mdaniel 11y ago
  Ah, that's what I came to the comments to find out, having just heard about an Android vulnerability in Chrome. The way the news of that vuln read (that one just needs to visit a web page) I thought this vuln would certainly fit that description.
asveikau 11y ago
The libpng simple decoder sample is pretty reasonable C, except for the fact that it uses setjmp to handle errors. (libpng's fault, not the sample.)
snogglethorpe 11y ago
libpng is actually very easy to use directly. 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.
- AndyKelley 11y ago
  Are you referring to setjmp? Because it's quite the opposite. This blog article does a good job explaining: http://latentcontent.net/2007/12/05/libpng-worst-api-ever/ http://latentcontent.net/2007/12/05/libpng-worst-api-ever/
  - snogglethorpe 11y ago
    Huh? I'm not referring to setjmp at all... (why would you think I'm referring to setjmp?) 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.
    - AndyKelley 11y ago
      Ah, I thought you were referring to setjmp with this: > 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 :-).

Asbostos 11y ago

There's now an idea that that robustness principle was backwards. The classic example being web browsers which were so liberal they encouraged all sorts of weird non-standard web pages which now have to be supported forever. 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.