|
Bugzilla – Full Text Bug Listing |
| Summary: | VUL-0: CVE-2004-0597: libpng: nasty security hole in libpng | ||
|---|---|---|---|
| Product: | [Novell Products] SUSE Security Incidents | Reporter: | Thomas Biege <thomas> |
| Component: | Incidents | Assignee: | Juergen Weigert <jw> |
| Status: | RESOLVED FIXED | QA Contact: | Security Team bot <security-team> |
| Severity: | Critical | ||
| Priority: | P3 - Medium | CC: | gnome-bugs, mls, nadvornik, postadal, security-team, stark |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | CVSSv2:NVD:CVE-2004-0597:10.0:(AV:N/AC:L/Au:N/C:C/I:C/A:C) | ||
| Found By: | --- | Services Priority: | |
| Business Priority: | Blocker: | --- | |
| Marketing QA Status: | --- | IT Deployment: | --- |
| Attachments: |
libpng-patch.diff
patchinfo-box.libpng patchinfo.libpng libpng vendor-sec discussion bad_images.tar |
||
|
Description
Thomas Biege
2004-07-14 20:23:29 UTC
<!-- SBZ_reproduce --> - Date: Tue, 13 Jul 2004 21:08:43 +0100 (BST) From: Mark J Cox <mjc@redhat.com> To: chris@scary.beasts.org Cc: vendor-sec@lst.de Subject: Re: [vendor-sec] Nasty security hole in libpng > I'm not sure what to do with the advisory; libpng doesn't seem > particularly maintained. Who did people liase with in the case of the > recent libpng problems? Ideas appreciated. Hi Chris; with the last libpng issue that was reported to us we just worked with the other vendor-sec folks and came up with a suitable embargo date. It looks like this issue is going to affect a whole load of packages, do you have any particular timescale in mind for disclosure? Here are some CVE names > 1) Remotely exploitable stack-based buffer overrun in png_handle_tRNS > (pngrutil.c) > 2) Dangerous code in png_handle_sBIT (pngrutil.c) (Similar code in > png_handle_hIST). CAN-2004-0597 for these (we merge issues that have the same flaw type that get fixed in the same versions). > 3) Possible NULL-pointer crash in png_handle_iCCP (pngrutil.c) (this > flaw is duplicated in multiple other locations). CAN-2004-0598 for those > 4) Theoretical integer overflow in allocation in png_handle_sPLT > (pngrutil.c) > 5) Integer overflow in png_read_png (pngread.c) > 6) Integer overflows during progressive reading. > 7) Other flaws. [integer overflows] CAN-2004-0599 for those Cheers, Mark _________________ Created attachment 22160 [details]
libpng-patch.diff
CAN-2004-0597 (stack overflows) is definately needed CAN-2004-0598 (NULL-pointer crashes) sounds definately needed CAN-2004-0599 (integer overflows) since there were many of these outlined I figured at least one might have a security context. Created attachment 22164 [details]
patchinfo-box.libpng
Created attachment 22165 [details]
patchinfo.libpng
Date: Wed, 14 Jul 2004 13:38:14 -0400 From: Matthias Clasen <mclasen@redhat.com> To: vendor-sec@lst.de Cc: chris@scary.beasts.org Subject: Re: [vendor-sec] Nasty security hole in libpng Parts/Attachments: 1 Shown 40 lines Text 2 10 KB Application ---------------------------------------- Hey Chris, some comments on your patch: 1) the pngpread.c changes look fine, I came up with exactly the same after reading your initial mail. 2) the image dimension checks should probably be done in png_set_IHDR - there are already checks against PNG_MAX_UINT there, which can be modified to be a bit stricter. 3) Your additional checks against png_ptr->channels > 4 and png_ptr->num_palette > PNG_MAX_PALETTE_LENGTH are not really necessary, since libpng already enforces this. But you have to study png_handle_IHDR, png_set_IHDR and png_handle_PLTE carefully to determine this, so adding the extra checks may still be a good idea for clarity. 4) Here is a fix for the theoretical overflow in png_handle_sPLT which you mention: --- libpng-1.2.5/pngrutil.c.chunklength2002-10-03 07:32:30.000000000 -0400 +++ libpng-1.2.5/pngrutil.c 2004-07-14 12:27:03.000000000 -0400 @@ -1154,6 +1154,8 @@ } new_palette.nentries = data_length / entry_size; + if (new_palette.nentries > PNG_MAX_UINT / sizeof(png_sPLT_entry)) + png_error(png_ptr, "sPLT chunk too long"); new_palette.entries = (png_sPLT_entryp)png_malloc( png_ptr, new_palette.nentries * sizeof(png_sPLT_entry)); 5) I have constructed a crasher image for the "possible NULL pointer crash" which you mention as issue 3. My crasher image causes a segfault in png_push_handle_tEXT() when loaded incrementally. I'm attaching it wrapped in a tar file, to keep the email from crashing Evolution... Regards, Matthias Clasen (RH libpng package maintainer) ate: Wed, 14 Jul 2004 20:33:09 +0100 (BST) From: chris@scary.beasts.org To: Matthias Clasen <mclasen@redhat.com> Cc: vendor-sec@lst.de Subject: Re: [vendor-sec] Nasty security hole in libpng Hi! On Wed, 14 Jul 2004, Matthias Clasen wrote: > some comments on your patch: ... > 3) Your additional checks against png_ptr->channels > 4 and > png_ptr->num_palette > PNG_MAX_PALETTE_LENGTH are not really necessary, > since libpng already enforces this. But you have to study > png_handle_IHDR, png_set_IHDR and png_handle_PLTE carefully to determine > this, so adding the extra checks may still be a good idea for clarity. Yep, I agree. In my advisory, I noted that it seems to not be exploitable and just a case of bad code. At one stage I thought it _was_ exploitable if a very specific set of circumstances was met - but it seems not. The lack of exploitability is IMHO "luck" so I'd be more comfortable keeping the redundant check in place. It will guard against future code changes accidentally opening up a hole. > 4) Here is a fix for the theoretical overflow in png_handle_sPLT which > you mention: I don't think this is needed because of the added checks against all PNG chunk lengths being <= PNG_UINT_MAX (which is actually defined as the C standard INT_MAX). Running through this function with length set at PNG_UINT_MAX, I don't think the overflow can trigger. Let me know if I'm wrong. > 5) I have constructed a crasher image for the "possible NULL pointer > crash" which you mention as issue 3. My crasher image causes a segfault > in png_push_handle_tEXT() when loaded incrementally. I'm attaching it > wrapped in a tar file, to keep the email from crashing Evolution... Hey, excellent. I'm glad I wasn't smoking regarding this one. The libpng maintainer has responded, and is working towards a libpng-1.2.6 with all the known security issues fixed. He is also making tweaks to my patch. You might want to contact him Glenn Randers-Pehrson <glennrp@glennrp.com> and work together to get the best combined patch. Cheers Chris 1. A stack-based buffer overflow in libpng which can be triggered to run arbitrary code by a malicious png file: CAN-2004-0597 2. A NULL-pointer crash in libpng which can be triggered by a malicious png file: CAN-2004-0598 3. Various possible integer overflows in libpng which may have security consequences: CAN-2004-0599 Date: Thu, 15 Jul 2004 09:51:10 -0400 From: Matthias Clasen <mclasen@redhat.com> To: chris@scary.beasts.org Cc: vendor-sec@lst.de Subject: Re: [vendor-sec] Nasty security hole in libpng On Wed, 2004-07-14 at 15:33, chris@scary.beasts.org wrote: > > 4) Here is a fix for the theoretical overflow in png_handle_sPLT which > > you mention: > > I don't think this is needed because of the added checks against all PNG > chunk lengths being <= PNG_UINT_MAX (which is actually defined as > the C standard INT_MAX). Running through this function with length set > at PNG_UINT_MAX, I don't think the overflow can trigger. Let me know if > I'm wrong. > Hmm, I guess that depends on how the compiler packs structs. length can be no larger than 2^31-1, thus slength and data_length are also <= 2^31-1. Thus newpalette.nentries can be no larger than 2^31-1/6 = 0x15555555. Now if sizeof (png_sPLT_entry) is 10, we end up allocating not more than 0xD5555552 < 2^32 bytes, but if the size of the struct is 20 (is the compiler actually allowed to do that ?), then we may end up allocating 0x1AAAAAAA4 > 2^32 bytes and overflow. Matthias Date: Thu, 15 Jul 2004 17:06:37 +0100 (BST) From: chris@scary.beasts.org To: Matthias Clasen <mclasen@redhat.com> Cc: vendor-sec@lst.de Subject: Re: [vendor-sec] Nasty security hole in libpng Hi, On Thu, 15 Jul 2004, Matthias Clasen wrote: > Hmm, I guess that depends on how the compiler packs structs. length > can be no larger than 2^31-1, thus slength and data_length are also > <= 2^31-1. Thus newpalette.nentries can be no larger than > 2^31-1/6 = 0x15555555. Now if sizeof (png_sPLT_entry) is 10, we end up > allocating not more than 0xD5555552 < 2^32 bytes, but if the size of the > struct is 20 (is the compiler actually allowed to do that ?), then > we may end up allocating 0x1AAAAAAA4 > 2^32 bytes and overflow. Good point - better to be safe than sorry. Cheers Chris Thomas, are the patches from comment #3 and comment #7 sufficient? CRD: 04.08.2004 Created attachment 22251 [details]
libpng vendor-sec discussion
Created attachment 22252 [details]
bad_images.tar
concerning comment #12, yes I think so. Please have a look at the whole discussion attached in comment #14 because Tom Rini <trini@mvista.com> had some trouble backporting the patch. Packages and patchinfo files are submitted. What about all other packages that contain libpng? They are probably also vulnerable if they don't use the system's libpng. Please check. - abiword - abiword2 - crossover-office - crossover-plugin - doxygen - ghostpcl - htmldoc - qt3 - rrdtool - tetex - tkimg - wxGTK There is also - AROS - MainActor - Mozilla - MozillaFirebird - MozillaThunderbird - amaya - chronium - jazz I'll lookup the maintainers... Hello Maintainers, I added you to CC to cjeck if your package does NOT use the system-wide libpng. Please add your comments here. qt3 has a patch to use the system wide one. abiword* probably uses system one (must check for older versions, too) chromium, ghostpcl and jazz probably uses its own (but ghostpcl distribution was cancelled by legal team) we still support SL 8.1 upwards, so it's not needed to check versions prior 8.1. i have verified crossover-office and crossover-plugin, while png is mentioned in the logfile, it is neither compiled nor linked in. htmldoc uses the system wide one. tkimg uses it's own. I have to check if I can patch it to use the one from the system, or if I have to replace the one that comes with the package. all mozilla packages since 8.1 use --with-system-png. So they really should not be statically linked with old libs. For 8.0 I'm not sure but we don't care about it anymore. MainActor is vulnerable and cannot be fixed easily. They use a binary only libgfl.so.1.93 to read images, which obviously contains libpng. They need to disable PNG support through libgfl and implement an alternative using system libpng. I'll have vendor check their options. just a private side-note from CERT:
Hello folks,
We have received the following additional information from the libpng
maintainers regarding the vulnerabilities we reported to you last
week:
A test version of libpng is available that deals with these
vulnerabilit
ies.
See http://libpng.sourceforge.net/scary/
It's in two formats, take your pick:
libpng-1.2.6beta3b.tar.gz
lp126b3b.zip
It incorporates the patch from the bug report, except for the
PNG_MAX_DIMENSION change which has been replaced with a check
inside the png_read_png() function (VU#160448).
Feel free to pass this information along to any libpng vendors
with whom you are communicating. Ask them not to make any links
to the above-mentioned directory so it isn't picked up prematurely
by search engines.
Just as a reminder, the original reporter has indicated that he will
publish his advisory regarding these issues on August 4, 2004. We
intend to be prepared to publish information about these issues at
that time. Please respond with any comments or vendor statements you
might have before that date.
If you have any additional questions or concerns, please do not
hesitate to contact us.
Best Regards,
Chad
--
Chad Dougherty
jazz: Code is not used. ghostpcl: Code is not used. xv, xemacs, gnuplot, and ghostscript use system libpng mainactor: upstream promised an update that uses system libpng before Aug 4. wxGTK uses system library. Ok, here my summary: package system own action ------------------------------------------------------------------------------- abiword + - na abiword2 + - na crossover-office + - na crossover-plugin + - na doxygen ? ? NO RESPONSE ghostpcl - + (code not used) na htmldoc + - na qt3 + - na rrdtool ? ? NO RESPONSE tetex ? ? NO RESPONSE tkimg - + !!!UPDATE!!! wxGTK + - na AROS ? ? NO RESPONSE MainActor - + !!!UPDATE!!! Mozilla + - na MozillaFirebird + - na MozillaThunderbird + - na amaya ? ? NO RESPONSE chronium - + !!!UPDATE!!! jazz - + (code not used) stable xv + - na xemacs + - na gnuplot + - na ghostscript + - na I've no response for the following packages: - AROS: stephan@suse.de - amaya: ltinkl@suse.cz - doxygen: postadal@suse.cz - textex: ??? - rddtools: ??? Updates neeed for 4th of August: - tkimg - MainActor - chronium jazz, what do you think about using the system libpng in future? ok, the packages is called tetex and not textex. and it seems to use the systemwide libpng since 8.1. chromium uses libpng to read it's own datafiles. It does not open any png from untrusted source. I don't think the update is needed. I've just submitted tkimg to STABLE and 9.1. It didn't exist in earlier versions. It uses libpng, libtiff, and libjpeg from the system instead of it's own now. Shall I write the patchinfo file or is the security team going to do that? AFAICS no one of the tools of tetex uses its own libpng but only the system libpng. "rddtools" was "rrdtool" in comment #18 and is maintained by tcrhak@suse.cz rrdtool in SL 9.1 and STABLE uses the system libpng 8.0, ..., 9.0 uses its own libpng-1.0.9 I just learned that it is not good to introduce a new package dependency (on libpng) with a YOU update. But it would be possible to put the tkimg rpm into the same patchinfo file as libpng. Cornelius Schumacher told me it is possible to have multiple RPMs in one patchinfo file but only those that are already installed will be updated. Can we do that for libpng and tkimg, and maybe also for the other packages that get changed to use the system libpng due to this bug? That won't help. The new dependency would only hurt if libpng is not installed in the system, in that case libpng will also not be installed with the update. So what else would you suggest, Michael? Can we just assume that libpng exists on any sensible system that has X11 (which is in turn requird by tkimg)? The "clean" solution would be to apply the png security fixes to the png sources of the package. I think the included version is too old for that: libpng version 1.0.8 - July 24, 2000 So the realistic options are (in order of my personal preferrence): a) Use the current version and hope nobody who has installed tkimg is missing libpng (I think chances are very good, as libpng is in the standard installation and required by many other packages and tkimg is optional). b) Make a patchinfo file for tkimg that contains and installs libpng as well (the libpng update might be downloaded twice in this case). c) Use the current version, but link it statically. Awaiting further advice... d) a YOU popup window for telling the customer about the dependency c) sounds good to me. no popups please JFYI: Date: Wed, 28 Jul 2004 19:27:21 +0100 (BST) From: chris@scary.beasts.org To: vendor-sec@lst.de Subject: [vendor-sec] Re: New libpng vulnerabilities: libpng-1.2.6beta4, beta4d (fwd) Hi, This is a libpng update. CERT requested more time beyond 4th Aug, but after discussion with the libpng maintainer, this request has been denied. The libpng community were strongly against further delay. So as far as I'm aware we're still on for Aug 4th. Hopefully nothing will leak until then, although more libpng people are about to be brought into the loop. Also note that some libpng people want to do an immediate disclosure but I think they have been disuaded. I'm forwarding details of the latest + greatest official fixed libpng release below in case you find it useful. Cheers Chris ---------- Forwarded message ---------- Date: Wed, 28 Jul 2004 13:59:05 -0400 From: "Glenn Randers-Pehrson <glennrp>" <glennrp@comcast.net> To: chris@scary.beasts.org Subject: Re: New libpng vulnerabilities: libpng-1.2.6beta4, beta4d chris@scary.beasts.org wrote: > > Hi Glenn, > > On Wed, 28 Jul 2004, Glenn Randers-Pehrson <glennrp> wrote: > > > I have decided that I will open the discussion to all of png-implement > > tonight or tomorrow regardless of the CERT request. Greg can decide > > whether to shut off or delay the archiving process or not. I will see > > about producing a supplemental archive of the off-list messages on the > > topic. > > Could you remind me of the location of the latest fully fixed libpng? I'll > let vendor-sec know in case of an early leak. > > Cheers > Chris http://www.graphicsmagick.org/libpng/beta/src # libpng-1.2.6beta4e sources http://www.graphicsmagick.org/libpng/beta/patches # patches for all versions also a.k.a. http://www.simplesystems.org/libpng/beta/* Glenn _______________________________________________ Vendor Security mailing list Vendor Security@lst.de https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec The patch for libpng 1.0.8 from http://www.graphicsmagick.org/libpng/beta/patches applies well to rrdtool's libpng 1.0.9 (there is no patch for 1.0.9 there). Is that single patch sufficient? The patch looks complete. OK, so I'll use that one for tkimg, too. tkimg and patchinfo file submitted for 9.1. rrdtool 8.0, 8.1, 8.2 and 9.0 submitted is sles7 still maintained? Tomas: can I have a patchinfo for rrdtool ? submitted rrdtool.box and rrdtool.maintained comment #57, sles7 isnt maintained anymore. Package Doxygen uses own libpng for generating own diagrams and doesn't use it for any untrusted sources, I think the update for doxygen isn't needed. Petr, ok. But can you update it for stable please. please take care about this: Date: Tue, 3 Aug 2004 23:33:05 -0400 (EDT) From: glennrp@studio.imagemagick.org To: Simon Cooper <scooper@apple.com> Cc: vendor-sec@lst.de Subject: Re: [vendor-sec] libpng patches. Warning: the "libpng-<ver>-all-patches.txt" files are missing patch11* On Tue, 3 Aug 2004, Simon Cooper wrote: > For those patching older versions of libpng I wanted warn everyone that > the "libpng-<ver>-all-patches.txt" files > are all missing "patch11". > > The combined patch files have not been updated since July 23rd and > patch11 appears to have been added early on Aug 2nd. > The combined patch files and the associated INFO.txt file have been updated; all now contain patch11. See http://www.graphicsmagick.org/libpng/beta/patches Glenn _______________________________________________ Vendor Security mailing list Vendor Security@lst.de https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec Please have a look at your source (libpng, tkimg, rrdtool, jazz, chronium, MainActor) if patch http://www.graphicsmagick.org/libpng/beta/patches/ libpng-patch11-limit-dimensions.txt is missing. If this is the case, we should release the current update package and make another update later. The patch from comment #3, which I used for libpng has similar check in png_handle_IHDR, the difference is that the dimensions are not checked on image write. Is it sufficient? For chromium, did you notice comment #40? about comment #40: yes I saw it, but it would be nice to have it fixed at least in STABLE anyway. comment #65: reading is more prone to be exploited, but nevertheless is should be fixed for writing too. packages approved. please handle the missing patch. Thank you! sorry.. sounds a bit rude. can you handle the missing patch please? :) I submitted libpng with the official patches. Thomas, can you please submit patchinfos? done. rrdtool and libpng approved.
Still open: opera (we wait for a new release from opera software)
and MainActor
Jürgen, is there a new version available now?
I've submitted an update of MainActor-5 to STABLE. This is a binary package. It works with 9.1, but may not work with 9.0 or before. (package MainActor, which contains a 3.x version is internal). Thanks! I consider this bug fixed now. |