### Summary
An exploitable integer overflow vulnerability exists in the tiff_image_parse functionality of Gdk-Pixbuf 2.36.6 when compiled with Clang. 
A specially crafted tiff file can cause a heap-overflow resulting in remote code execution. An attacker can send a file or a URL to trigger this vulnerability. 


### Tested Versions

Gdk-Pixbuf 2.36.6 commit: aba8d88798dfc2f3856ea0ddda14b06174bbb2bc
compiled with clang -O3 flag
libtiff 4.0.6

### Product URLs
[https://developer.gnome.org/gdk-pixbuf/](https://developer.gnome.org/gdk-pixbuf/)


### CVSSv3 Score

8.8 - CVSS:3.0/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H

### CWE
CWE-758: Reliance on Undefined, Unspecified, or Implementation-Defined Behavior

### Details
	Gdk-Pixbuf is a toolkit for image loading and pixel buffer manipulation used in various type of desktop applications: image viewers(GNOME thumbnailer), web browsers (Chromium, Firefox), media players (VLC), etc.
The vulnerability exists in the TIFF parser and only manifests itself when the library is compiled with high optimization flags `-O3` (tested with Clang, gcc does not remove the check). Several defined `if statements` 
inside the `tiff_image_parse` function  are responsible of integer overflow checks or at least that was their intention. Because the checks are made on signed integers, the condition cannot evaluate to false unless an 
integer overflow occurs.  According to the C standard, a signed integer overflow is defined as "Undefined Bahavior", thus behaviour related to it is implementation dependent and in the case of Clang the check is removed.
Finally the lack of proper integer overflows check leads to heap overflow and can allow attackers to obtain arbitrary code execution.

The code below is removed from compilation process because it would be true if a `signed integer overflow` would occur which is `undefined bahavior`:

	Line 89         gint width, height, rowstride, bytes;
	Line 128        rowstride = width * 4;
	Line 129        if (rowstride / 4 != width) { /* overflow */
	Line 130                g_set_error_literal (error,
	Line 131                                     GDK_PIXBUF_ERROR,
	Line 132                                     GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
	Line 133                                     _("Dimensions of TIFF image too large"));
	Line 134                return NULL;                
	Line 135        }
	Line 136        
	Line 137        bytes = height * rowstride;
	Line 138        if (bytes / rowstride != height) { /* overflow */
	Line 139                g_set_error_literal (error,
	Line 140                                     GDK_PIXBUF_ERROR,
	Line 141                                     GDK_PIXBUF_ERROR_CORRUPT_IMAGE,
	Line 142                                     _("Dimensions of TIFF image too large"));
	Line 143                return NULL;                
	Line 144        }


in our case the variables have the following values:

	width  = 0x8020
	height = 0xfff7
	
which causes an integer overflow at `line 137`:
	
	bytes = height * ( width * 4)
    
Then, based on the overflowed value, a buffer is allocated :

	Line 160       pixels = g_try_malloc (bytes);
	
Then all three parameters: width,height and the allocated `bytes` buffer are passed as arguments:

	Line 272		if (!TIFFReadRGBAImageOriented (tiff, width, height, (uint32 *)pixels, ORIENTATION_TOPLEFT, 1)) {	
	
Because buffer `bytes` was allocated based on overflowed value, width and height parameters mismatch the size of the buffer which leads to out of bound writes (Line 1362) inside the `put1bitbwtile` function while reading RGB values:


	libtiff/tif_getimage.c:1326
	Line 1351	/*
	Line 1352	 * 1-bit bilevel => colormap/RGB
	Line 1353	 */
	Line 1354	DECLAREContigPutFunc(put1bitbwtile)
	Line 1355	{
	Line 1356		uint32** BWmap = img->BWmap;
	Line 1357
	Line 1358		(void) x; (void) y;
	Line 1359		fromskew /= 8;
	Line 1360		while (h-- > 0) {
	Line 1361		uint32* bw;
	Line 1362		UNROLL8(w, bw = BWmap[*pp++], *cp++ = *bw++);
	Line 1363		cp += toskew;
	Line 1364		pp += fromskew;
	Line 1365		}
	Line 1366	}	

	
### Crash Information 

	valgrind ./pixbuf-read crashes/tiff_bug.tiff
	==32378== Invalid write of size 4
	==32378==    at 0x4B5D71D: put1bitbwtile (tif_getimage.c:1326)
	==32378==    by 0x4B5BE5E: gtTileContig (tif_getimage.c:673)
	==32378==    by 0x4B5B810: TIFFRGBAImageGet (tif_getimage.c:495)
	==32378==    by 0x4B5B8ED: TIFFReadRGBAImageOriented (tif_getimage.c:514)
	==32378==    by 0x4067A59: tiff_image_parse (io-tiff.c:272)
	==32378==    by 0x406634C: gdk_pixbuf__tiff_image_stop_load (io-tiff.c:477)
	==32378==    by 0x4048442: gdk_pixbuf_loader_close (gdk-pixbuf-loader.c:822)
	==32378==    by 0x8048903: test_loader (pixbuf-read.c:35)
	==32378==    by 0x8048903: main (pixbuf-read.c:75)
	==32378==  Address 0x5343ba8 is 0 bytes after a block of size 7,207,808 alloc'd
	==32378==    at 0x402D17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
	==32378==    by 0x432C3BF: g_try_malloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4800.2)
	==32378==    by 0x4067584: tiff_image_parse (io-tiff.c:160)
	==32378==    by 0x406634C: gdk_pixbuf__tiff_image_stop_load (io-tiff.c:477)
	==32378==    by 0x4048442: gdk_pixbuf_loader_close (gdk-pixbuf-loader.c:822)
	==32378==    by 0x8048903: test_loader (pixbuf-read.c:35)
	==32378==    by 0x8048903: main (pixbuf-read.c:75)

### Credit 

Discovered by Marcin 'Icewall' Noga of Cisco Talos.

I believe this is bug 780269.
The patch (attachment 348263 [details] [review]) changes the preconditions of exactly the two multiplications mentioned here in this bug report.

Feel free to unmark as duplicate if my analysis is wrong.

*** This bug has been marked as a duplicate of bug 780269 ***

Where's the reproducer for this one?

Created attachment 355516 [details]
crashing file, password "crash"

This works as expected when compiled with GCC, like 99% of Linux distributions. I'll add the test case to git now.

commit 3b8559efaa0907c8b9a1ae5e6db7f62cca331c62
Author: Bastien Nocera <hadess@hadess.net>
Date:   Thu Jul 13 17:06:12 2017 +0200

    tests: Add a test image for extra large dimensions in TIFF loader
    
    Overflow checks were added in commit 14904cb842.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784903

(In reply to Bastien Nocera from comment #5)
> This works as expected when compiled with GCC, like 99% of Linux
> distributions. I'll add the test case to git now.

yeah, it's undefined behaviour. Compilers can do what they want. GCC may be fine in this very case. Although it's clear that they regard these types of overflow check as wrong as this bug report shows: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30475#c10

So the checks introduced in commit 14904cb842 are not good according to GCC people.  It's undefined behaviour after all.  Checks that do not rely on undefined behaviour are proposed in bug 780269.

A similar issue is in bug 776694, I think.

(In reply to Tobias Mueller from comment #7)
> (In reply to Bastien Nocera from comment #5)
> > This works as expected when compiled with GCC, like 99% of Linux
> > distributions. I'll add the test case to git now.
> 
> yeah, it's undefined behaviour. Compilers can do what they want. GCC may be
> fine in this very case. Although it's clear that they regard these types of
> overflow check as wrong as this bug report shows:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30475#c10
> 
> So the checks introduced in commit 14904cb842 are not good according to GCC
> people.  It's undefined behaviour after all.  Checks that do not rely on
> undefined behaviour are proposed in bug 780269.

Fair enough, but that bug report is still 5 years younger than the commit for add those overflow checks. "Wrong" is also relative. It's undefined, but seems to have worked in a defined way for at least 15 years ;)

> A similar issue is in bug 776694, I think.

I'll look as well.

Duplicate of a public bug, making public.