GNOME Bugzilla – Bug 358117
jpeg plugin crashes when opening certain jpg file (crash in libexif)
Last modified: 2008-01-15 14:08:59 UTC
The jpeg plugin crashes when opening a certain jpg file (attached). If relevant: system libjpeg62 version is 6b-11.
Created attachment 73548 [details] GIMP message saying the plugin has crashed
Created attachment 73549 [details] Image causing the jpeg plugin to crash
Looks a lot like a bug in libexif. I can reproduce the crash:
+ Trace 73484
This image is from an Olympus C-2500L camera, a 2.5 MPixel digital SLR from 1999 or 2000, and the unusual thing about it is that the jpeg files it creates appear to lack thumbnails. I have never seen a jpeg file with exif but no thumbnail, and maybe the libexif developers haven't either, because it looks like the code is reading a bogus data size and then trying to do a bogus realloc based on it, leading to the failure.
This should be reported to the libexif developers then so that they can take appropriate action. Bill, do you think there's anything simple we can do in the JPEG plug-in to work around this problem? Otherwise, we can only ask the bug reporter to inform the libexif guys and close this report as NOTGNOME.
If my understanding is correct, then we can work around the problem by avoiding any attempt to load the exif for images that lack a thumbnail. I have tested by adding the lines /* return if there is no thumbnail, to work around bug #358117 */ if (!exif_data->data || exif_data->size == 0) return; to the function jpeg_apply_exif_data_to_image() in jpeg-exif.c, and the change does allow this image to load successfully (losing the exif, however). Of course as you say, the proper solution would be to fix this within libexif, but I think the workaround can't do much harm, since exif images without thumbnails are very rare.
Looks good, but this check could probably be in the metadata plug-in, couldn't it? There it would catch the problem also for other plug-ins using gimp_metadata_store_exif(). And we might even be do this in a way that doesn't loose all the EXIF data.
Well, the problematic code only comes into play when we format the exif data into a parasite to attach to the image. The metadata plug-in is supposed to replace this step eventually, but currently we don't have any other way of saving the exif data. The metadata plug-in will do it by translating the exif data into XMP/XML, which is nontrivial -- if it weren't, I or Raphael would already have done it. (If this response doesn't make sense, it is probably because I didn't correctly understand what you are suggesting.)
Well, according to the stack trace, the crash happens in gimp_metadata_store_exif() and this looked like metadata code to me. I forgot that this is still compiled into the JPEG plug-in. Perhaps we need to clean this up a bit before 2.4?
Wow, this was really fast! Shall I report this issue to the libexif developer? Maybe it's better when you do this because I don't have much experience with this library and how it works together with the jpeg plugin.
Please go ahead an report the bug to libexif (http://sourceforge.net/projects/libexif), making sure to give a pointer to this bug report. Their maintenance doesn't seem to be very active, so I'm not sure how much response there will be. I will commit the change from comment #6, so at least your files will be openable. Regarding comment #9, I'm not sure what my attitude is -- the code does look a bit odd, but on the other hand any changes made now are going to have to be unwound once a proper implementation of gimp_metadata_store_exif exists. For HEAD: 2006-09-29 Bill Skaggs <weskaggs@primate.ucdavis.ed> * plug-ins/jpeg/jpeg-exif.c: don't let libexif crash us when loading image with exif data but no thumbnail. Works around bug #358117. Resolving as NOTGNOME.
Oh, this was already reported to the libexif bug tracker: https://sourceforge.net/tracker/index.php?func=detail&aid=1525770&group_id=12272&atid=112272 But indeed it doesn't seem very active...
*** Bug 381182 has been marked as a duplicate of this bug. ***
Although the problem is not in our code, I am tentatively re-opening this bug report in case we can find a workaround before GIMP 2.4 is released. It does not seem very likely that everybody will have an updated version of libexif in time for the release.
Bill added a workaround for this bug (see comment #11). Thus I don't see a need to keep it open, closing as NOTGNOME again.
Unfortunately, the workaround is not sufficient. It is still possible to get a crash (or libexif hanging and consuming insane amounts of memory) with the image attached to bug #428037 (attachment #86111 [details]).
I tried applying the patch from https://sourceforge.net/tracker/index.php?func=detail&aid=1457501&group_id=12272&atid=112272 to libexif, but it doesn't fix our problem. Note that the fix in the bug report is also not complete as the code continues with j=25 which is out of bounds of the elem array.
*** Bug 324302 has been marked as a duplicate of this bug. ***
This bug report and the bug #1255770 at sf.net actually mix 3 images. I was not able to reproduce any crash on the 1st two images. I just added a heuristical fix to deal with the 3rd image (attachment #86111 [details]). Note: all three images have corrupted EXIF data. Could you please try GIMP with the latest libexif from CVS? Details here: https://sourceforge.net/tracker/index.php?func=detail&aid=1525770&group_id=12272&atid=112272
There should really be a way to tell libexif to avoid even looking at EXIF fields that GIMP is not interested in. GIMP does not use MakerNote and most of the crashes that have been reported recently were related to libexif getting stuck while parsing MakerNote.
Modifying EXIF but not modifying MakerNote is not a good idea. Many MakerNotes actually use offsets relative to the start of EXIF marker and not makernote. Many tools do ignore the MakerNote and therefore we have zillions of corrupted files around. For an example, go to bug #428037. However this morning I checked in an enhancement to libexif: Saving unmodified makernote no longer controlled at compile time by #ifdef EXIF_DONT_CHANGE_MAKER_NOTE, but at run-time by newly introduced option EXIF_DATA_OPTION_DONT_CHANGE_MAKER_NOTE.
The crashers seem to have gone with libexif 0.6.15. I can't reproduce the crashes with the user-provided test cases which used to crash before. I'm attaching the test cases to this bug so that people can test more easily, as some critical crashers are in bugs marked as dups and are not easily findable.
Created attachment 89589 [details] Crasher image 1 which loads fine with libexif 0.6.15 and GIMP trunk
Created attachment 89590 [details] Crasher image 2 which loads fine with libexif 0.6.15 and GIMP trunk
Created attachment 89591 [details] Crasher image 3 which loads fine with libexif 0.6.15 and GIMP trunk
Created attachment 89592 [details] Crasher image 4 which loads fine with libexif 0.6.15 and GIMP trunk
Bumped required libexif version to 0.6.15 in changeset 22752. Closing bug (raphael's killer duck image also works now). 2007-06-11 Mukund Sivaraman <muks@mukund.org> * configure.in: bumped required libexif version to 0.6.15. This seems to fix #358117 for the various test images that were reported in it, and its duplicates. This version is in Debian testing and Fedora 7.
*** Bug 456645 has been marked as a duplicate of this bug. ***
The EXIF reading code has been reorganized in SVN (revision 22949). The new code is simpler and successfully loads all images attached here. The workaround described in comment #6 has been removed because we depend on a more robust libexif now. This allows the plug-in to read EXIF blocks without a thumbnail image.
*** Bug 460466 has been marked as a duplicate of this bug. ***
Note for those who assign duplicates: There is a new bug #466044 that also occurs with a photo taken by an Olympus camera, but that bug is different because it affects libexif 0.6.16 and it causes it to allocate endless amounts of memory instead of crashing. Please pay attention to the differences and assign the duplicates correctly. Thanks in advance.
*** Bug 473556 has been marked as a duplicate of this bug. ***
*** Bug 484809 has been marked as a duplicate of this bug. ***
*** Bug 486620 has been marked as a duplicate of this bug. ***