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 780269 - CVE-2017-2870 Gdk-Pixbuf TIFF tiff_image_parse Code Execution Vunerability (integer overflow)
CVE-2017-2870 Gdk-Pixbuf TIFF tiff_image_parse Code Execution Vunerability (i...
Status: RESOLVED FIXED
Product: gdk-pixbuf
Classification: Platform
Component: loaders
git master
Other Linux
: Normal normal
: ---
Assigned To: gdk-pixbuf-maint
gdk-pixbuf-maint
CVE-2017-2870
: CVE-2017-2870 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-03-19 16:58 UTC by Tobias Mueller
Modified: 2017-08-25 19:41 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
patch (1.75 KB, patch)
2017-03-19 16:58 UTC, Tobias Mueller 		needs-work Details | Review
example file (336 bytes, image/tiff)
2017-03-19 16:59 UTC, Tobias Mueller 		  Details
added the crashing file (1007 bytes, patch)
2017-07-13 14:24 UTC, Tobias Mueller 		committed Details | Review
patch with more descriptive message (2.17 KB, patch)
2017-07-13 14:44 UTC, Ludovico de Nittis 		none Details | Review
tiff: Check for integer overflows in multiplication (2.30 KB, patch)
2017-07-13 16:58 UTC, Bastien Nocera 		committed Details | Review

Description Tobias Mueller 2017-03-19 16:58:38 UTC 
Created attachment 348263 [details] [review]
patch

I seems the current checks for multiplication overflows rely on undefined behaviour are are thus not reliable.  afl produced a file that makes the tiff loader code behave undefinedly (according the UBSan):

io-tiff.c:127:19: runtime error: signed integer overflow: 1073741824 * 4 cannot be represented in type 'int'

Attached is a patch by Ludovico which makes the issue disappear.  The patch looks innocent enough to me.  Too sad that we don't have a g_int_checked_mul().

I tried to find out whether coverity also complains, but I can't access it atm, probably due to maintenance work on their side.
Comment 1 Tobias Mueller 2017-03-19 16:59:43 UTC 
Created attachment 348264 [details]
example file
Comment 2 Tobias Mueller 2017-04-07 09:00:59 UTC 
I probably shouldn't have opened this bug report because there is already a bug report: bug 770986. Now even with  an alternative patch.
Comment 3 Tobias Mueller 2017-07-13 13:27:08 UTC 
*** Bug 784903 has been marked as a duplicate of this bug. ***
Comment 4 Bastien Nocera 2017-07-13 14:07:18 UTC 
Review of attachment 348263 [details] [review]:

Commit message needs work.
Comment 5 Bastien Nocera 2017-07-13 14:09:00 UTC 
Comment on attachment 348264 [details]
example file

Example file should be in a git formatted patch.
Comment 6 Tobias Mueller 2017-07-13 14:24:49 UTC 
Created attachment 355519 [details] [review]
added the crashing file
Comment 7 Ludovico de Nittis 2017-07-13 14:44:33 UTC 
Created attachment 355524 [details] [review]
patch with more descriptive message

Amended the patch with a more informative commit message.
Comment 8 Bastien Nocera 2017-07-13 16:58:44 UTC 
Created attachment 355537 [details] [review]
tiff: Check for integer overflows in multiplication

The checks currently in use are not sufficient, because they depend on
undefined behaviour:

    rowstride = width * 4;
    if (rowstride / 4 != width) { /* overflow */

If the multiplication has already overflowed, the compiler may decide
to optimize the if out and thus we do not handle the erroneous case.

Rearrange the checks to avoid the undefined behaviour.

Note that gcc doesn't seem to be impacted, though a defined behaviour is
obviously preferred.

CVE-2017-2870
Comment 9 Bastien Nocera 2017-07-13 17:09:13 UTC 
Attachment 355537 [details] pushed as 31a6cff - tiff: Check for integer overflows in multiplication