So, after some debugging on CVE-2018-7456, I start to get the feeling to understand the issue.
Here are my conclusions for the moment:
* In any case, the transfer function should not care about other
channels defined by ExtraSamples (see 2nd and 3rd paragraphs of
TransferFunction entry, page 84 of the specification), so something
like the following patch should be a first step:
--- a/libtiff/tif_print.c 2018-03-17 21:56:47.000000000 +0100
+++ b/libtiff/tif_print.c 2018-03-17 22:05:58.446092016 +0100
@@ -546,7 +546,7 @@
uint16 i;
fprintf(fd, " %2ld: %5u",
l, td->td_transferfunction[0][l]);
- for (i = 1; i < td->td_samplesperpixel; i++)
+ for (i = 1; i < td->td_samplesperpixel -
td->td_extrasamples; i++)
fprintf(fd, " %5u",
td->td_transferfunction[i][l]);
fputc('\n', fd);
But it's not enough. Why ?
* I discovered that td->td_samplesperpixel can have arbitrary size while
td->td_extrasamples stays 0. It shouldn't be the case ! For instance
the specification doesn't allow RGB with 4 channels and no ExtraSamples.
And while it doesn't make any sense, libtiff won't notice it.
I even tried RGB with 8 channels and no ExtraSamples and it worked too.
So, what should be done ?
For each PhotometricInterpretation type there is a 'standard' samples
per pixel value (that is the samples per pixel value without extra samples:
3 for RGB, etc). Let's name it (standard spp).
So, what we should do is: If the actual td->td_samplesperpixel is higher
than (standard spp), then we should assume that td->td_extrasamples is
td->td_samplesperpixel - (standard spp), even if no ExtraSamples field
was specified OR if the specified td->td_extrasamples was smaller.
Obviously we should also take care of filling appropriate
td->td_sampleinfo entries.
For example, if an RGB image has td->td_samplesperpixel = 4, then
td->td_extrasamples should be set to 1 (with arbitrary
td->td_sampleinfo entry for example 0 - Unspecified data).
Does it work now ?
I think so ! I didn't write the second part of the patch and will wait
for feedback. But you can convince yourself that it doesn't crash anymore
by modifying the sample to add an ExtraSamples = 1 field (using
"tiffset -s 338 1 0 $(sample)" for example).
Link to the specification:
https://www.itu.int/itudoc/itu-t/com16/tiff-fx/docs/tiff6.pdf
Regards,
Hugo
--
Hugo Lefeuvre (hle) | www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA
signature.asc
Description: PGP signature
