Bug#929597: [PATCH] CVE-2019-12211: heap buffer overflow via memcpy

[Anton Gladky]

Thanks, Hugo, for analyzing the issue in details and proposing the fix.

Do you want to add the patch into the corresponding forum-thread
in freeimage website?

Regards

Anton

Am Sa., 26. Okt. 2019 um 16:11 Uhr schrieb Hugo Lefeuvre <hle at debian.org>:
>
> Hi,
>
> The overflow happens during the following call to memcpy:
>
>     // convert to strip
>     if(x + tileWidth > width) {
>             src_line = imageRowSize - rowSize;
>     } else {
>             src_line = tileRowSize;
>     }
>     BYTE *src_bits = tileBuffer;
>     BYTE *dst_bits = bits + rowSize;
>     for(int k = 0; k < nrows; k++) {
>             memcpy(dst_bits, src_bits, src_line);
>             src_bits += tileRowSize;
>             dst_bits -= dst_pitch;
>     }
>
> This portion of code copies image data from a libTIFF-provided buffer to an
> internal buffer. The overflow happens because src_line is larger than the
> size of dst_bits.
>
> This is the result of an inconsistency between libTIFF and freeimage:
>
> In the libTIFF case, tile row size is
> = samplesperpixel * bitspersample * tilewidth / 8
> = bitsperpixel * tilewidth / 8
> = 6 * 32 * 7 / 8 = 168
>
> In the freeimage case, tile row size is
> bitsperpixel * tilewidth / 8
> = 32 * 7 / 8 = 28
>
> As a result, the two buffers are differently sized.
>
> freeimage has a bpp of 32 because CreateImageType calls
> FreeImage_AllocateHeader with MIN(bpp, 32).
>
> This 'MIN(bpp, 32)' looks like a terrible hack to me, but we can't change
> it to 'bpp' because FIT_BITMAP images with bpp > 32 does not seem to be
> supported by freeimage. Also, in this case, bpp > 32 doesn't even make
> sense:
>
> Looking closely at the reproducer, we can notice that it defines a bilevel
> image with samplesperpixel and bitspersample parameters, both unexpected in
> bilevel images.
>
> Pixels in bilevel images can either be black or white. There is as such
> only one sample per pixel, and a single bit per sample is sufficient.  The
> spec defines bpp = 8. It is unclear whether the specification allows for
> arbitrary values of bitspersample or samplesperpixel (extrasamples?) in
> this case.
>
> This file gets rejected by most libTIFF tools.
>
> # patch
>
> + add check to CreateImageType() to reject FIT_BITMAP images with bpp > 32
>   instead of passing MIN(bpp, 32).
> + change type of dst_pitch to unsigned
> + call memcpy with MIN(dst_pitch, src_line) instead of src_line. this will
>   help overcome any further (future) discrepancy between libTIFF and
>   freeimage.
>
> # tests
>
> I have tested for regressions with the following samples, using a modified
> version of Examples/Linux/linux-gtk.c:
>
> http://www.simplesystems.org/libtiff/images.html
>
> During these tests, I found other issues with bilevel images, unrelated to
> this patch. I will try to take a look at them in the future.
>
> I can provide additional explanations if there is anything unclear.
>
> I'd like to get this patch peer-reviewed/merged upstream before shipping
> it in a Debian release.
>
> regards,
> Hugo
>
> --
>                 Hugo Lefeuvre (hle)    |    www.owl.eu.com
> RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
> ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C