|
Lucio Andrés Illanes Albornoz |
daac7c |
From e1cd2d7ab032e7fe80b4c13e07895194c8bac85e Mon Sep 17 00:00:00 2001
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
From: Brian May <brian@linuxpenguins.xyz>
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
Date: Thu, 7 Dec 2017 07:46:47 +1100
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
Subject: [PATCH 1/4] [PATCH] tiff2pdf: Fix CVE-2017-9935
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
Fix for http://bugzilla.maptools.org/show_bug.cgi?id=2704
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
This vulnerability - at least for the supplied test case - is because we
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
assume that a tiff will only have one transfer function that is the same
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
for all pages. This is not required by the TIFF standards.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
We than read the transfer function for every page. Depending on the
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
transfer function, we allocate either 2 or 4 bytes to the XREF buffer.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
We allocate this memory after we read in the transfer function for the
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
page.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
For the first exploit - POC1, this file has 3 pages. For the first page
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
we allocate 2 extra extra XREF entries. Then for the next page 2 more
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
entries. Then for the last page the transfer function changes and we
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
allocate 4 more entries.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
When we read the file into memory, we assume we have 4 bytes extra for
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
each and every page (as per the last transfer function we read). Which
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
is not correct, we only have 2 bytes extra for the first 2 pages. As a
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
result, we end up writing past the end of the buffer.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
There are also some related issues that this also fixes. For example,
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
TIFFGetField can return uninitalized pointer values, and the logic to
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
detect a N=3 vs N=1 transfer function seemed rather strange.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
It is also strange that we declare the transfer functions to be of type
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
float, when the standard says they are unsigned 16 bit values. This is
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
fixed in another patch.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
This patch will check to ensure that the N value for every transfer
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
function is the same for every page. If this changes, we abort with an
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
error. In theory, we should perhaps check that the transfer function
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
itself is identical for every page, however we don't do that due to the
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
confusion of the type of the data in the transfer function.
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
---
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
libtiff/tif_dir.c | 3 +++
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
tools/tiff2pdf.c | 69 +++++++++++++++++++++++++++++++----------------
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
2 files changed, 49 insertions(+), 23 deletions(-)
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
diff --git a/libtiff/tif_dir.c b/libtiff/tif_dir.c
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
index f00f808..c36a5f3 100644
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
--- a/libtiff/tif_dir.c
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+++ b/libtiff/tif_dir.c
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
@@ -1067,6 +1067,9 @@ _TIFFVGetField(TIFF* tif, uint32 tag, va_list ap)
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
if (td->td_samplesperpixel - td->td_extrasamples > 1) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
*va_arg(ap, uint16**) = td->td_transferfunction[1];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
*va_arg(ap, uint16**) = td->td_transferfunction[2];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ } else {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ *va_arg(ap, uint16**) = NULL;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ *va_arg(ap, uint16**) = NULL;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
}
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
break;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
case TIFFTAG_REFERENCEBLACKWHITE:
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
diff --git a/tools/tiff2pdf.c b/tools/tiff2pdf.c
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
index bdb9126..bd23c9e 100644
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
--- a/tools/tiff2pdf.c
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+++ b/tools/tiff2pdf.c
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
@@ -239,7 +239,7 @@ typedef struct {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
float tiff_whitechromaticities[2];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
float tiff_primarychromaticities[6];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
float tiff_referenceblackwhite[2];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- float* tiff_transferfunction[3];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ uint16* tiff_transferfunction[3];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
int pdf_image_interpolate; /* 0 (default) : do not interpolate,
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
1 : interpolate */
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
uint16 tiff_transferfunctioncount;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
@@ -1049,6 +1049,8 @@ void t2p_read_tiff_init(T2P* t2p, TIFF* input){
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
uint16 pagen=0;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
uint16 paged=0;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
uint16 xuint16=0;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ uint16 tiff_transferfunctioncount=0;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ uint16* tiff_transferfunction[3];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
directorycount=TIFFNumberOfDirectories(input);
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
if(directorycount > TIFF_DIR_MAX) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
@@ -1157,26 +1159,48 @@ void t2p_read_tiff_init(T2P* t2p, TIFF* input){
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
}
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
#endif
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
if (TIFFGetField(input, TIFFTAG_TRANSFERFUNCTION,
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- &(t2p->tiff_transferfunction[0]),
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- &(t2p->tiff_transferfunction[1]),
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- &(t2p->tiff_transferfunction[2]))) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- if((t2p->tiff_transferfunction[1] != (float*) NULL) &&
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- (t2p->tiff_transferfunction[2] != (float*) NULL) &&
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- (t2p->tiff_transferfunction[1] !=
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->tiff_transferfunction[0])) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->tiff_transferfunctioncount = 3;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->tiff_pages[i].page_extra += 4;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->pdf_xrefcount += 4;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- } else {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->tiff_transferfunctioncount = 1;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->tiff_pages[i].page_extra += 2;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->pdf_xrefcount += 2;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- }
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- if(t2p->pdf_minorversion < 2)
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->pdf_minorversion = 2;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ &(tiff_transferfunction[0]),
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ &(tiff_transferfunction[1]),
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ &(tiff_transferfunction[2]))) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ if((tiff_transferfunction[1] != (uint16*) NULL) &&
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ (tiff_transferfunction[2] != (uint16*) NULL)
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ ) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ tiff_transferfunctioncount=3;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ } else {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ tiff_transferfunctioncount=1;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ }
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
} else {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->tiff_transferfunctioncount=0;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ tiff_transferfunctioncount=0;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
}
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ if (i > 0){
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ if (tiff_transferfunctioncount != t2p->tiff_transferfunctioncount){
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ TIFFError(
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ TIFF2PDF_MODULE,
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ "Different transfer function on page %d",
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ i);
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->t2p_error = T2P_ERR_ERROR;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ return;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ }
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ }
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->tiff_transferfunctioncount = tiff_transferfunctioncount;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->tiff_transferfunction[0] = tiff_transferfunction[0];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->tiff_transferfunction[1] = tiff_transferfunction[1];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->tiff_transferfunction[2] = tiff_transferfunction[2];
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ if(tiff_transferfunctioncount == 3){
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->tiff_pages[i].page_extra += 4;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->pdf_xrefcount += 4;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ if(t2p->pdf_minorversion < 2)
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->pdf_minorversion = 2;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ } else if (tiff_transferfunctioncount == 1){
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->tiff_pages[i].page_extra += 2;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->pdf_xrefcount += 2;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ if(t2p->pdf_minorversion < 2)
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ t2p->pdf_minorversion = 2;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ }
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
if( TIFFGetField(
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
input,
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
TIFFTAG_ICCPROFILE,
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
@@ -1837,10 +1861,9 @@ void t2p_read_tiff_data(T2P* t2p, TIFF* input){
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
&(t2p->tiff_transferfunction[0]),
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
&(t2p->tiff_transferfunction[1]),
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
&(t2p->tiff_transferfunction[2]))) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- if((t2p->tiff_transferfunction[1] != (float*) NULL) &&
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- (t2p->tiff_transferfunction[2] != (float*) NULL) &&
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- (t2p->tiff_transferfunction[1] !=
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
- t2p->tiff_transferfunction[0])) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ if((t2p->tiff_transferfunction[1] != (uint16*) NULL) &&
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ (t2p->tiff_transferfunction[2] != (uint16*) NULL)
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
+ ) {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
t2p->tiff_transferfunctioncount=3;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
} else {
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
t2p->tiff_transferfunctioncount=1;
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
--
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
2.17.0
|
|
Lucio Andrés Illanes Albornoz |
daac7c |
|