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 <h...@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