Menu

#53 exif crashes when removing thumbnail from jpeg image

None
closed-fixed
exif (14)
5
2018-06-30
2006-08-07
mulhern
No

Under certain situations exif, while loading jpeg data
from a file, will fail to set a pointer. Later, when
writing out the modified image to a new file it will
crash when it dereferences this pointer, still NULL.

This bug was uncovered as part of the Cooperative Bug
Isolation Project <http://www.cs.wisc.edu/cbi>, which
attempts to find bugs by discovering statistically
significant differences in the behavior of good and bad
runs.

["We" here refers to the members of the Cooperative Bug
Isolation research team. Though just one of us is
submitting this report, it is the product of a joint
research effort to which all members made invaluable
contributions.]

This bug was discovered because it was unexplained in
our test cases; i.e., our tools could not determine its
cause as being identical to one of the other bugs
previously reported. The causes of the bug were
discovered by running exif with the "-r" command flag
on a number of images; some repeated instances of the
image that had been discovered to cause the bug in the
first place as well as many images that were known not
to be the source of the bug.

This bug manifest under exif-0.6.9 and either
libexif-0.6.13 or libexif-0.6.10.

A test image, "input.jpg" is available at
http://www.cs.wisc.edu/~mulhern/input.jpg.
We did not attach the file since it exceeds the size
limits. The command "exif -r input.jpg" causes a
segmentation fault. A typical stack trace looks as follows:

Program received signal SIGSEGV, Segmentation fault.
0x009096bc in memcpy () from /lib/tls/libc.so.6
(gdb) bt
#0 0x009096bc in memcpy () from /lib/tls/libc.so.6
#1 0x08057329 in jpeg_data_save_data (data=0x97fe290,
d=0xbffaeeb4,
ds=0xbffaeeb8) at jpeg-data.c:156
#2 0x080575cf in jpeg_data_save_file (data=0x8067a60,
path=0xbffaf020 "input.jpg.modified.jpeg") at
jpeg-data.c:87
#3 0x0804da86 in save_exif_data_to_file (ed=0x97fe2b0,
fname=0xbffd7a5b "input.jpg", target=0xbffaf020
"input.jpg.modified.jpeg")
at main.c:186
#4 0x0804fa30 in main (argc=3, argv=0xbffaf684) at
main.c:449

Our analysis tools direct us to the predicate o < s in
method jpeg_data_load_data() at line 233 of jpeg_data.c.

Further inspection shows us the mechanism behind the bug.

We inspect a stack trace taken at a point where the 8th
and last JPEGSection is being read in.

(gdb) bt
#0 jpeg_data_load_data (data=0x8bb51f8, d=0xb7f05008
"????m?Exif",
size=460886) at jpeg-data.c:234
#1 0x08057966 in jpeg_data_load_file (data=0x8bb51f8,
path=0x0)
at jpeg-data.c:306
#2 0x080579ac in jpeg_data_new_from_file (path=0x0) at
jpeg-data.c:270
#3 0x0804d29f in save_exif_data_to_file (ed=0x8bad448,
fname=0xbff01a46 "input.jpg", target=0xbfeba460
"input.jpg.modified.jpeg")
at main.c:163
#4 0x0804ebaa in main (argc=3, argv=0xbfebaac4) at
main.c:567

Memory for the JPEGSection has been allocated and its
values are being set.

(gdb) print *s
$20 = {marker = JPEG_MARKER_SOF3, content = {generic =
{data = 0x0,
size = 4082511165}, app1 = 0x0}}

At this point its data field and its app1 field are
both null; each JPEGSection is "cleaned up" in this way
in a previous step.

However, since o > s the condition (o + len > s) at
line 234 in jpeg_data.c evaluates to true and the
function jpeg_data_load_data() returns early leaving
the data and app1 pointers NULL.

After manipulating the image, exif begins to write out
the modified image. When writing out the 8th and last
JPEGSection exif reaches the default case of the switch
statement in jpeg_data_save_data() at line 150 in
jpeg_data.c. At line 156 memcpy() segfaults since the
data field in the JPEGSection struct is still NULL.

We suggest that a fix for this bug is some action other
than just an early return at line 234 in jpeg-data.c.

Discussion

  • Marcus Meissner

    Marcus Meissner - 2006-10-03

    Logged In: YES
    user_id=48092

    the error is likely still there in 0.6.13, according to the
    code.

     
  • Jan Patera

    Jan Patera - 2007-05-08

    Logged In: YES
    user_id=943941
    Originator: NO

    Unfortunately the sample image is no longewr available at http://www.cs.wisc.edu/~mulhern/input.jpg

     
  • Jan Patera

    Jan Patera - 2007-05-08

    Logged In: YES
    user_id=943941
    Originator: NO

    Unfortunately the sample image is no longewr available at http://www.cs.wisc.edu/~mulhern/input.jpg

     
  • Dan Fandrich

    Dan Fandrich - 2018-06-30

    I'm attaching a file that seems to show the issue here, although it doesn't give me a SIGSEGV (yet) but does show a lot of Conditional jump or move depends on uninitialised value(s) errors under valgrind.

    The underlying issue was solved in commit 72e9fb36 by ensuring the size parameter is initialized to 0.

     
  • Dan Fandrich

    Dan Fandrich - 2018-06-30
    • status: open --> closed-fixed
    • assigned_to: Dan Fandrich
    • Group: -->
     

Log in to post a comment.