#20 exif crashes when printing NULL string in "-m" mode
Synopsis: exif crashes when printing NULL string
When exif is run with the "-m" flag to produce machine-readable output, it occasionally encounters a NULL string. When show_entry_machine() subsequently tried to print this NULL string, exif crashes. We are seeing this crash using exif-0.6.9 with libexif-0.6.11.
This bug was uncovered as part of the Cooperative Bug Isolation Project <http://sampler.cs.berkeley.edu/>, which attempts to find bugs by discovering statistically significant differences in the behavior of good and bad runs. This bug, along with several others, was discovered by running exif with random command line flags on random elements of a large corpus of JPEG images. We observed a 7% crash rate across the entire experiment. The bug we describe here accounts for 74% of those failures.
["We" here refers to the members of the Cooperative Bug Isolation research team at Stanford University and the University of California, Berkeley. 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.]
We are attaching a test image, "image1.jpg", to this report. The command "exif -m image1.jpg" causes a segmentation fault. A typical stack trace looks as follows:
#0 0x4207a8a3 in strlen () from /lib/tls/libc.so.6
#1 0x42062572 in fputs () from /lib/tls/libc.so.6
#2 0x0804ce94 in show_entry_machine (e=0x8070bc8, data=0x0) at actions.c:172
#3 0x4001d43c in exif_content_foreach_entry (content=0x806f0d0, func=0x804cdfc <show_entry_machine>, data=0xbfffd843) at exif-content.c:164
#4 0x0804cede in show_ifd_machine (content=0x806f0d0, data=0xbfffd843) at actions.c:179
#5 0x4002bd9e in exif_data_foreach_content (data=0x806eff8, func=0x804cec8 <show_ifd_machine>, user_data=0xbfffd843) at exif-data.c:874
#6 0x0804cf1c in action_tag_list_machine (filename=0xbffffa9a "image1.jpg", ed=0x806eff8, ids=0 '\0') at actions.c:187
#7 0x08050605 in main (argc=3, argv=0xbfffdeb4) at main.c:570
#8 0x42015704 in __libc_start_main () from /lib/tls/libc.so.6
The cause of this bug is that function exif_entry_get_value in file libexif-0.6.10/libexif/exif-entry.c returns a NULL value, which is not checked in the body of function show_entry_machine in file exif-0.6.9/exif/actions.c:
show_entry_machine(...)
{
...
fprintf (stdout, "%s", exif_entry_get_value (e, v, sizeof (v)));
...
}
glibc guarantees that fprintf works correctly when asked to print a NULL value (it prints the string "(null)"). However, in the above case, GCC performs a strength reduction optimization: it replaces the call to fprintf by a call to fputs. glibc does *not* guarantee that fputs works correctly when asked to print a NULL value. Indeed, in the above case, when fputs is asked to print a NULL value returned by exif_entry_get_value, it crashes in strlen (which is called by fputs). Arguably this is a GCC optimizer bug. However, it is never wise to rely on the behavior of fprintf when asked to print NULL strings.
The fix for this bug is to print the result of exif_entry_get_value only when it is non-NULL:
show_entry_machine(...)
{
...
char *s = exif_entry_get_value (e, v, sizeof (v));
if (s) fprintf (stdout, "%s", s);
...
}
Indeed, our experimental infrastructure does not attribute any crashes in our scripted runs to this bug once the above fix is introduced.
Also, we believe the declaration:
char *v[1024];
in function show_entry_machine should be:
char v[1024];
This is not a bug but it seems from reading the code of function show_entry_machine that the programmer intended the latter.
Also, there is another statement in function exif_entry_dump in file libexif-0.6.10/libexif/exif-entry.c that calls exif_entry_get_value and prints its result without checking whether it is non-NULL:
exif_entry_dump(...)
{
...
printf ("%s Value: %s\n", buf, exif_entry_get_value (e, value, sizeof(value)));
...
}
The fix is the same as the one suggested above.
As noted earlier, this bug was found using tools which are part of the Cooperative Bug Isolation Project. The stack trace shown above, for instance, provides no indication that exif_entry_get_value was even called. Since our system tracks the values of predicates in the program, it directs the programmer's attention to the last line of the function exif_entry_get_value:
return strlen (val) ? val : NULL;
Specifically, it directs the programmer's attention to the predicate that captures the fact that exif_entry_get_value returns NULL. Once this fact is known, it is straightforward for the programmer to infer that fputs is crashing because it is being asked to print a NULL value returned by exif_entry_get_value in the body of show_entry_machine.
test image demonstrating the bug
Logged In: YES
user_id=2116
Patch 1005652 <http://sourceforge.net/tracker/index.php?func=detail&aid=1005652&group_id=12272&atid=312272>, which was independently submitted by another person, should fix the primary issue described in this bug report.
Please note, however, that there are two additional problems reported here. There's another instance of printing a possibly-null string in exif_entry_dump(), and there's a benign but incorrect variable declaration in show_entry_machine(). I realize it's bad form to put multiple problems in a single report; sorry about that.
Logged In: YES
user_id=58652
See #1051994.