After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 775648 - integer underflow in qtif
integer underflow in qtif
Status: RESOLVED FIXED
Product: gdk-pixbuf
Classification: Platform
Component: loaders
git master
Other Linux
: Normal normal
: ---
Assigned To: gdk-pixbuf-maint
gdk-pixbuf-maint
Depends on:
Blocks:
 
 
Reported: 2016-12-05 16:08 UTC by Tobias Mueller
Modified: 2016-12-13 18:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
crashing file, password "crash", found by afl (200 bytes, application/pgp-encrypted)
2016-12-05 16:08 UTC, Tobias Mueller
  Details
potential patch (1.21 KB, patch)
2016-12-05 16:10 UTC, Tobias Mueller
rejected Details | Review
qtif: Avoid buffer overrun on short reads (1.13 KB, patch)
2016-12-13 18:21 UTC, Bastien Nocera
committed Details | Review
tests: Add test for bug 775648 (870 bytes, patch)
2016-12-13 18:22 UTC, Bastien Nocera
committed Details | Review

Description Tobias Mueller 2016-12-05 16:08:49 UTC
Created attachment 341409 [details]
crashing file, password "crash", found by afl

I think the attached file crashes gdk-pixbuf:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff0ef7d79 in gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x62100100918c "", size=4278255618, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb)

We can see how size underflows:


Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x62100001914e "", size=64, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) c
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x62100001914f "@idatP5 332 922", size=63, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) c
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x621000019150 "idatP5 332 922", size=62, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) c
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x621000019151 "datP5 332 922", size=61, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) 
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x621000019152 "atP5 332 922", size=60, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) c
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x621000019153 "tP5 332 922", size=59, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) 
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x62100001918c "", size=2, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) 
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x62100001918d "\377", size=1, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) 
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x62100001918e "", size=0, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) 
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x62100001918f "", size=4294967295, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) 
Continuing.

Breakpoint 3, gdk_pixbuf__qtif_image_load_increment (data=0x60700000af20, buf=0x621000019190 "", size=4294967294, error=0x7fffffffd830) at io-qtif.c:437
437	                context->header_buffer[context->run_length] = *buf;
(gdb) 
Continuing.



I have cooked up an emergency patch which checks for the underflow. The check for the sanity of that size argument should probably be performed earlier, somewhere, but I don't know gdk-pixbuf enough.
Comment 1 Tobias Mueller 2016-12-05 16:10:11 UTC
Created attachment 341411 [details] [review]
potential patch
Comment 2 Bastien Nocera 2016-12-13 18:21:58 UTC
Created attachment 341905 [details] [review]
qtif: Avoid buffer overrun on short reads

When filling the QTIF buffer, stop looping when we've copied 'size'
bytes, not when the buffer is filled. This fixes out-of-bounds accesses
when size is shorter than the expected header.
Comment 3 Bastien Nocera 2016-12-13 18:22:04 UTC
Created attachment 341906 [details] [review]
tests: Add test for bug 775648
Comment 4 Bastien Nocera 2016-12-13 18:22:58 UTC
After fixing it to avoid running past the end of the loop, the file
is actually readable.

Attachment 341905 [details] pushed as 92ac5e8 - qtif: Avoid buffer overrun on short reads
Attachment 341906 [details] pushed as b1fbea9 - tests: Add test for bug 775648