Bug 2778 - A NULL Pointer Dereference in libtiff (CVE-2018-7456)
: A NULL Pointer Dereference in libtiff (CVE-2018-7456)
Status: RESOLVED FIXED
: libtiff
default
: unspecified
: PC Linux
: P2 enhancement
: ---
Assigned To:
:
:
:
:
:
  Show dependency treegraph
 
Reported: 2018-02-22 20:55 by
Modified: 2018-04-12 15:11 (History)


Attachments
Patch for CVE-2018-7456 (3.39 KB, patch)
2018-03-21 12:23, Hugo Lefeuvre
Details | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2018-02-22 20:55:32
A NULL Pointer Dereference in function TIFFPrintDirectory in tif_print.c when
using tiffinfo tool to print the crafted tiff information.

Affected  Version : v4.09, including the latest commit  cad3e7d8 from
https://gitlab.com/libtiff/libtiff


$ tiffinfo -c $FILE

ASAN:SIGSEGV
=================================================================
==172==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc
0x7f3b8f83a44f bp 0x000000000000 sp 0x7ffd7cacd6b0 T0)
    #0 0x7f3b8f83a44e in TIFFPrintDirectory
/src/libtiff/libtiff/tif_print.c:549
    #1 0x402329 in tiffinfo /src/libtiff/tools/tiffinfo.c:461
    #2 0x402329 in main /src/libtiff/tools/tiffinfo.c:150
    #3 0x7f3b8f2ce82f in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #4 0x402888 in _start (/src/aflbuild/installed/bin/tiffinfo+0x402888)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/libtiff/libtiff/tif_print.c:549
TIFFPrintDirectory
==172==ABORTING

the poc refer to https://github.com/xiaoqx/pocs/tree/master/libtiff.
------- Comment #1 From 2018-03-12 16:31:19 -------
Problem happens here, while processing the transfer function:
https://gitlab.com/libtiff/libtiff/blob/master/libtiff/tif_print.c#L549

I wonder if this is similar to a previous bug in tiff2pdf.c:
http://bugzilla.maptools.org/show_bug.cgi?id=2704
------- Comment #2 From 2018-03-18 03:57:35 -------
A patch is currently in development at https://bugs.debian.org/891288, feedback
is welcome. I'll come back here with the finished version.
------- Comment #3 From 2018-03-21 12:23:42 -------
Created an attachment (id=848) [details]
Patch for CVE-2018-7456

Here are my conclusions on this bug along with some comments on the attached
patch:

The TIFFPrintDirectory relies on the following assumptions supposed to be
guaranteed by the specification:

(a) A Transfer Function field is only present if the TIFF file has photometric
type < 3.

(b) If SamplesPerPixel > Color Channels, then the ExtraSamples field has count
SamplesPerPixel - (Color Channels) and contains information about supplementary
channels.

While the respect of (a) and (b) are essential for the functioning of
TIFFPrintDirectory, no checks are realized neither by the callee nor by
TIFFPrintDirectory itself. Hence, following scenarios might happen and trigger
CVE-2018-7456:

(1) TIFF File of photometric type 4 or more has illegal Transfer Function
field.

(2) TIFF File has photometric type 3 or less and defines a SamplesPerPixel
field such that SamplesPerPixel > Color Channels without declaring
ExtraSamples.

In this patch, we are trying to fix both issues with respect of the following
principles:

(A) In the case of (1), the defined transfer table should be printed safely
even if it isn't 'legal'. This allows us to avoid expensive checks in
TIFFPrintDirectory. Also, it is quite possible that an alternative photometric
type would be developed (not part of the standard) and would allow definition
of Transfer Table. We want libtiff to be able to handle this scenario out of
the box.

(B) In the case of (2), the transfer table should be printed at its right size,
that is if TIFF file has photometric type Palette then the transfer table
should have one row and not three, even if two extra samples are declared.

Some comments on the patch:

- The for loop in TIFFPrintDirectory has now two end conditions: 'i <
td->td_samplesperpixel - td->td_extrasamples' addresses (B) and 'i < 3'
addresses (A).

- 'i < td->td_samplesperpixel - td->td_extrasamples' is useless if we don't
modify libtiff/tif_dirread.c to fix the ExtraSamples field: All non-color
channels should be defined as ExtraSamples.

- TIFFGetColorChannels doesn't handle all cases because I couldn't find any
reliable document describing the properties of LOGL, CFA or ITULAB. Feedback
welcome.

- Alternatively the 'i < td->td_samplesperpixel - td->td_extrasamples'
condition could be replaced by 'i < TIFFGetColorChannels(td->td_photometric)'
but while this would make the patch clearly smaller this is IMO not the best
idea because:

* TIFFPrintDirectory is a print function and should not contain too much logic

* Other parts of the source code may still rely on such assumptions and will
still be vulnerable if we put the check in TIFFPrintDirectory

You can find several test files covering different special cases in the Debian
bug report.