(CVE-2021-46829) Buffer overwrite in io-gif-animation.c composite_frame() (possibly exploitable)

I have been fuzzing gdk-pixbuf and found an interesting bug. I did limited research on it, but the bug appears to be an exploitable memory overwrite.

The bug is in composite_frame() in io-gif-animation.c, which I pasted below. It can be triggered by running gdk-pixbuf-pixdata with any of the poc files attached to this report.

		pixels = gdk_pixbuf_get_pixels (anim->last_frame_data);
		for (i = 0; i < n_indexes; i++) {
				guint8 index = index_buffer[i];
				guint x, y;
				int offset;

				if (index == frame->transparent_index)
						continue;

				x = i % frame->width + frame->x_offset;
				y = interlace_rows[i / frame->width] + frame->y_offset;
				if (x >= anim->width || y >= anim->height)
						continue;

				offset = y * gdk_pixbuf_get_rowstride (anim->last_frame_data) + x * 4;
				int rowstride = gdk_pixbuf_get_rowstride (anim->last_frame_data);
				printf("signed: y: %d x: %d height: %d width: %d rowstride: %d offset: %d\n", y, x, anim->height, anim->width, rowstride, offset);
				printf("unsigned: y: %u x: %u height: %u width: %u rowstride: %u offset: %u\n", y, x, anim->height, anim->width, rowstride, offset);
				printf("buf size: %u\n", gdk_pixbuf_get_byte_length (anim->last_frame_data));
				pixels[offset + 0] = frame->color_map[index * 3 + 0];
				pixels[offset + 1] = frame->color_map[index * 3 + 1];
				pixels[offset + 2] = frame->color_map[index * 3 + 2];
				pixels[offset + 3] = 255;
		}

Note that the printf were added by me to aid in debugging. The problem appears to be in the calculation of the offset variable. Actually there are two problems there:

1. because it is a signed int, it can overwrite when it's over INT32_MAX
2. even if it was as an unsigned int, given the calculations, it becomes quite easy to reach very large values

I am attaching two files here. The first is wrap_around.poc, which gives this result:

signed: y: 25926 x: 18759 height: 26725 width: 22016 rowstride: 88064 offset: -2011744996
unsigned: y: 25926 x: 18759 height: 26725 width: 22016 rowstride: 88064 offset: 2283222300
buf size: 2353510400
Segmentation fault

As you can see, we've wrapped around and gone into a negative number, which causes a negative offset access on the buffer. This can be further checked in gdb:

$r11   : 0x00007fff6b247010  →  0x0000000000000000
$r15   : 0xffffffff8817351c

   0x55555556603b <composite_frame+667> lea    eax, [r10+r10*2]
   0x55555556603f <composite_frame+671> movsxd rdx, eax
   0x555555566042 <composite_frame+674> movzx  edx, BYTE PTR [rcx+rdx*1]
 → 0x555555566046 <composite_frame+678> mov    BYTE PTR [r11+r15*1], dl
   0x55555556604a <composite_frame+682> lea    edx, [rax+0x1]
   0x55555556604d <composite_frame+685> mov    rcx, QWORD PTR [rbp+0x20]
   0x555555566051 <composite_frame+689> add    eax, 0x2
   0x555555566054 <composite_frame+692> movsxd rdx, edx
   0x555555566057 <composite_frame+695> cdqe

Here the value of $r11 is the pixels buffer, while r15 is the calculated output variable. Its value is 0xffffffff8817351c, which is exactly -2011744996 as printed out by my added printfs.

The second file I am attaching, more_trouble.poc, shows that this can happen even with a smaller buffer. Its output results in:

signed: y: 32896 x: 128 height: 65526 width: 18303 rowstride: 73212 offset: -1886584832
unsigned: y: 32896 x: 128 height: 65526 width: 18303 rowstride: 73212 offset: 2408382464
buf size: 502322216
Segmentation fault

I haven't tried to exploit this, but looks to me that it is definitely exploitable. We can write to an arbitrary memory address, which is calculated using the offset variable. What I am not sure is how controllable that offset variable is; its value depends on the X and Y and other factors, which might influence the buffer size. I'm sure you know the code better than me and will be able to definitely tell if this is fully exploitable or not.

I also did a naive fix, which works perfectly to avoid this problem, but I'm not sure if its the correct way to go about it. You just need to introduce the following check after the offset calculation and before it is used to access the pixels buffer:

		 gsize len = gdk_pixbuf_get_byte_length (anim->last_frame_data);
		 g_assert (offset + 3 <= len && offset > 0);

This seems to stop every memory overwrite from happening, here's the output of wrap_around.poc with the patch:

signed: y: 25926 x: 18759 height: 26725 width: 22016 rowstride: 88064 offset: -2011744996
unsigned: y: 25926 x: 18759 height: 26725 width: 22016 rowstride: 88064 offset: 2283222300
buf size: 2353510400
**
GdkPixbuf:ERROR:../gdk-pixbuf/io-gif-animation.c:390:composite_frame: assertion failed: (offset + 3 <= len && offset > 0)
Bail out! GdkPixbuf:ERROR:../gdk-pixbuf/io-gif-animation.c:390:composite_frame: assertion failed: (offset + 3 <= len && offset > 0)
Aborted

pocs.tar.xz