Upstream patch for CVE-2012-4447. This also covers an out-of-bounds-read possibility in the same file, which wasn't given a separate CVE. diff -Naur tiff-3.9.4.orig/libtiff/tif_pixarlog.c tiff-3.9.4/libtiff/tif_pixarlog.c --- tiff-3.9.4.orig/libtiff/tif_pixarlog.c 2010-06-08 14:50:42.000000000 -0400 +++ tiff-3.9.4/libtiff/tif_pixarlog.c 2012-12-10 15:50:14.421538317 -0500 @@ -117,9 +117,9 @@ if (n >= stride) { mask = CODE_MASK; if (stride == 3) { - t0 = ToLinearF[cr = wp[0]]; - t1 = ToLinearF[cg = wp[1]]; - t2 = ToLinearF[cb = wp[2]]; + t0 = ToLinearF[cr = (wp[0] & mask)]; + t1 = ToLinearF[cg = (wp[1] & mask)]; + t2 = ToLinearF[cb = (wp[2] & mask)]; op[0] = t0; op[1] = t1; op[2] = t2; @@ -136,10 +136,10 @@ op[2] = t2; } } else if (stride == 4) { - t0 = ToLinearF[cr = wp[0]]; - t1 = ToLinearF[cg = wp[1]]; - t2 = ToLinearF[cb = wp[2]]; - t3 = ToLinearF[ca = wp[3]]; + t0 = ToLinearF[cr = (wp[0] & mask)]; + t1 = ToLinearF[cg = (wp[1] & mask)]; + t2 = ToLinearF[cb = (wp[2] & mask)]; + t3 = ToLinearF[ca = (wp[3] & mask)]; op[0] = t0; op[1] = t1; op[2] = t2; @@ -183,9 +183,9 @@ if (n >= stride) { mask = CODE_MASK; if (stride == 3) { - t0 = ToLinearF[cr = wp[0]] * SCALE12; - t1 = ToLinearF[cg = wp[1]] * SCALE12; - t2 = ToLinearF[cb = wp[2]] * SCALE12; + t0 = ToLinearF[cr = (wp[0] & mask)] * SCALE12; + t1 = ToLinearF[cg = (wp[1] & mask)] * SCALE12; + t2 = ToLinearF[cb = (wp[2] & mask)] * SCALE12; op[0] = CLAMP12(t0); op[1] = CLAMP12(t1); op[2] = CLAMP12(t2); @@ -202,10 +202,10 @@ op[2] = CLAMP12(t2); } } else if (stride == 4) { - t0 = ToLinearF[cr = wp[0]] * SCALE12; - t1 = ToLinearF[cg = wp[1]] * SCALE12; - t2 = ToLinearF[cb = wp[2]] * SCALE12; - t3 = ToLinearF[ca = wp[3]] * SCALE12; + t0 = ToLinearF[cr = (wp[0] & mask)] * SCALE12; + t1 = ToLinearF[cg = (wp[1] & mask)] * SCALE12; + t2 = ToLinearF[cb = (wp[2] & mask)] * SCALE12; + t3 = ToLinearF[ca = (wp[3] & mask)] * SCALE12; op[0] = CLAMP12(t0); op[1] = CLAMP12(t1); op[2] = CLAMP12(t2); @@ -247,9 +247,9 @@ if (n >= stride) { mask = CODE_MASK; if (stride == 3) { - op[0] = ToLinear16[cr = wp[0]]; - op[1] = ToLinear16[cg = wp[1]]; - op[2] = ToLinear16[cb = wp[2]]; + op[0] = ToLinear16[cr = (wp[0] & mask)]; + op[1] = ToLinear16[cg = (wp[1] & mask)]; + op[2] = ToLinear16[cb = (wp[2] & mask)]; n -= 3; while (n > 0) { wp += 3; @@ -260,10 +260,10 @@ op[2] = ToLinear16[(cb += wp[2]) & mask]; } } else if (stride == 4) { - op[0] = ToLinear16[cr = wp[0]]; - op[1] = ToLinear16[cg = wp[1]]; - op[2] = ToLinear16[cb = wp[2]]; - op[3] = ToLinear16[ca = wp[3]]; + op[0] = ToLinear16[cr = (wp[0] & mask)]; + op[1] = ToLinear16[cg = (wp[1] & mask)]; + op[2] = ToLinear16[cb = (wp[2] & mask)]; + op[3] = ToLinear16[ca = (wp[3] & mask)]; n -= 4; while (n > 0) { wp += 4; @@ -342,9 +342,9 @@ if (n >= stride) { mask = CODE_MASK; if (stride == 3) { - op[0] = ToLinear8[cr = wp[0]]; - op[1] = ToLinear8[cg = wp[1]]; - op[2] = ToLinear8[cb = wp[2]]; + op[0] = ToLinear8[cr = (wp[0] & mask)]; + op[1] = ToLinear8[cg = (wp[1] & mask)]; + op[2] = ToLinear8[cb = (wp[2] & mask)]; n -= 3; while (n > 0) { n -= 3; @@ -355,10 +355,10 @@ op[2] = ToLinear8[(cb += wp[2]) & mask]; } } else if (stride == 4) { - op[0] = ToLinear8[cr = wp[0]]; - op[1] = ToLinear8[cg = wp[1]]; - op[2] = ToLinear8[cb = wp[2]]; - op[3] = ToLinear8[ca = wp[3]]; + op[0] = ToLinear8[cr = (wp[0] & mask)]; + op[1] = ToLinear8[cg = (wp[1] & mask)]; + op[2] = ToLinear8[cb = (wp[2] & mask)]; + op[3] = ToLinear8[ca = (wp[3] & mask)]; n -= 4; while (n > 0) { n -= 4; @@ -393,9 +393,9 @@ mask = CODE_MASK; if (stride == 3) { op[0] = 0; - t1 = ToLinear8[cb = wp[2]]; - t2 = ToLinear8[cg = wp[1]]; - t3 = ToLinear8[cr = wp[0]]; + t1 = ToLinear8[cb = (wp[2] & mask)]; + t2 = ToLinear8[cg = (wp[1] & mask)]; + t3 = ToLinear8[cr = (wp[0] & mask)]; op[1] = t1; op[2] = t2; op[3] = t3; @@ -413,10 +413,10 @@ op[3] = t3; } } else if (stride == 4) { - t0 = ToLinear8[ca = wp[3]]; - t1 = ToLinear8[cb = wp[2]]; - t2 = ToLinear8[cg = wp[1]]; - t3 = ToLinear8[cr = wp[0]]; + t0 = ToLinear8[ca = (wp[3] & mask)]; + t1 = ToLinear8[cb = (wp[2] & mask)]; + t2 = ToLinear8[cg = (wp[1] & mask)]; + t3 = ToLinear8[cr = (wp[0] & mask)]; op[0] = t0; op[1] = t1; op[2] = t2; @@ -630,10 +630,10 @@ return guess; } -static uint32 -multiply(size_t m1, size_t m2) +static tsize_t +multiply(tsize_t m1, tsize_t m2) { - uint32 bytes = m1 * m2; + tsize_t bytes = m1 * m2; if (m1 && bytes / m1 != m2) bytes = 0; @@ -641,6 +641,20 @@ return bytes; } +static tsize_t +add_ms(tsize_t m1, tsize_t m2) +{ + tsize_t bytes = m1 + m2; + + /* if either input is zero, assume overflow already occurred */ + if (m1 == 0 || m2 == 0) + bytes = 0; + else if (bytes <= m1 || bytes <= m2) + bytes = 0; + + return bytes; +} + static int PixarLogSetupDecode(TIFF* tif) { @@ -661,6 +675,8 @@ td->td_samplesperpixel : 1); tbuf_size = multiply(multiply(multiply(sp->stride, td->td_imagewidth), td->td_rowsperstrip), sizeof(uint16)); + /* add one more stride in case input ends mid-stride */ + tbuf_size = add_ms(tbuf_size, sizeof(uint16) * sp->stride); if (tbuf_size == 0) return (0); sp->tbuf = (uint16 *) _TIFFmalloc(tbuf_size); Upstream patch for CVE-2012-4564. diff -Naur tiff-3.9.4.orig/tools/ppm2tiff.c tiff-3.9.4/tools/ppm2tiff.c --- tiff-3.9.4.orig/tools/ppm2tiff.c 2010-06-08 14:50:44.000000000 -0400 +++ tiff-3.9.4/tools/ppm2tiff.c 2012-12-10 16:16:05.154045877 -0500 @@ -68,6 +68,17 @@ exit(-2); } +static tsize_t +multiply_ms(tsize_t m1, tsize_t m2) +{ + tsize_t bytes = m1 * m2; + + if (m1 && bytes / m1 != m2) + bytes = 0; + + return bytes; +} + int main(int argc, char* argv[]) { @@ -85,6 +96,7 @@ int c; extern int optind; extern char* optarg; + tsize_t scanline_size; if (argc < 2) { fprintf(stderr, "%s: Too few arguments\n", argv[0]); @@ -217,7 +229,8 @@ } switch (bpp) { case 1: - linebytes = (spp * w + (8 - 1)) / 8; + /* if round-up overflows, result will be zero, OK */ + linebytes = (multiply_ms(spp, w) + (8 - 1)) / 8; if (rowsperstrip == (uint32) -1) { TIFFSetField(out, TIFFTAG_ROWSPERSTRIP, h); } else { @@ -226,15 +239,31 @@ } break; case 8: - linebytes = spp * w; + linebytes = multiply_ms(spp, w); TIFFSetField(out, TIFFTAG_ROWSPERSTRIP, TIFFDefaultStripSize(out, rowsperstrip)); break; } - if (TIFFScanlineSize(out) > linebytes) + if (linebytes == 0) { + fprintf(stderr, "%s: scanline size overflow\n", infile); + (void) TIFFClose(out); + exit(-2); + } + scanline_size = TIFFScanlineSize(out); + if (scanline_size == 0) { + /* overflow - TIFFScanlineSize already printed a message */ + (void) TIFFClose(out); + exit(-2); + } + if (scanline_size < linebytes) buf = (unsigned char *)_TIFFmalloc(linebytes); else - buf = (unsigned char *)_TIFFmalloc(TIFFScanlineSize(out)); + buf = (unsigned char *)_TIFFmalloc(scanline_size); + if (buf == NULL) { + fprintf(stderr, "%s: Not enough memory\n", infile); + (void) TIFFClose(out); + exit(-2); + } if (resolution > 0) { TIFFSetField(out, TIFFTAG_XRESOLUTION, resolution); TIFFSetField(out, TIFFTAG_YRESOLUTION, resolution); Fix unsafe handling of DotRange and related tags. Back-port of upstream patch for CVE-2012-5581. (Note: I have not pushed this into upstream CVS for the 3.9 branch, because I'm not entirely convinced that it won't create application compatibility issues --- tgl) diff -Naur tiff-3.9.7.orig/libtiff/tif_dir.c tiff-3.9.7/libtiff/tif_dir.c --- tiff-3.9.7.orig/libtiff/tif_dir.c 2012-09-22 10:48:09.000000000 -0400 +++ tiff-3.9.7/libtiff/tif_dir.c 2012-12-13 13:39:20.448864070 -0500 @@ -494,32 +494,28 @@ goto end; } - if ((fip->field_passcount + if (fip->field_tag == TIFFTAG_DOTRANGE + && strcmp(fip->field_name,"DotRange") == 0) { + /* TODO: This is an evil exception and should not have been + handled this way ... likely best if we move it into + the directory structure with an explicit field in + libtiff 4.1 and assign it a FIELD_ value */ + uint16 v[2]; + v[0] = (uint16)va_arg(ap, int); + v[1] = (uint16)va_arg(ap, int); + _TIFFmemcpy(tv->value, v, 4); + } + else if (fip->field_passcount || fip->field_writecount == TIFF_VARIABLE || fip->field_writecount == TIFF_VARIABLE2 || fip->field_writecount == TIFF_SPP - || tv->count > 1) - && fip->field_tag != TIFFTAG_PAGENUMBER - && fip->field_tag != TIFFTAG_HALFTONEHINTS - && fip->field_tag != TIFFTAG_YCBCRSUBSAMPLING - && fip->field_tag != TIFFTAG_DOTRANGE - && fip->field_tag != TIFFTAG_WHITELEVEL) { + || tv->count > 1) { _TIFFmemcpy(tv->value, va_arg(ap, void *), tv->count * tv_size); } else { - /* - * XXX: The following loop required to handle - * TIFFTAG_PAGENUMBER, TIFFTAG_HALFTONEHINTS, - * TIFFTAG_YCBCRSUBSAMPLING and TIFFTAG_DOTRANGE tags. - * These tags are actually arrays and should be passed as - * array pointers to TIFFSetField() function, but actually - * passed as a list of separate values. This behaviour - * must be changed in the future! - */ - int i; char *val = (char *)tv->value; - for (i = 0; i < tv->count; i++, val += tv_size) { + assert( tv->count == 1 ); switch (fip->field_type) { case TIFF_BYTE: case TIFF_UNDEFINED: @@ -578,7 +574,6 @@ status = 0; break; } - } } } } @@ -869,24 +864,27 @@ *va_arg(ap, uint16*) = (uint16)tv->count; *va_arg(ap, void **) = tv->value; ret_val = 1; - } else { - if ((fip->field_type == TIFF_ASCII + } else if (fip->field_tag == TIFFTAG_DOTRANGE + && strcmp(fip->field_name,"DotRange") == 0) { + /* TODO: This is an evil exception and should not have been + handled this way ... likely best if we move it into + the directory structure with an explicit field in + libtiff 4.1 and assign it a FIELD_ value */ + *va_arg(ap, uint16*) = ((uint16 *)tv->value)[0]; + *va_arg(ap, uint16*) = ((uint16 *)tv->value)[1]; + ret_val = 1; + } else { + if (fip->field_type == TIFF_ASCII || fip->field_readcount == TIFF_VARIABLE || fip->field_readcount == TIFF_VARIABLE2 || fip->field_readcount == TIFF_SPP - || tv->count > 1) - && fip->field_tag != TIFFTAG_PAGENUMBER - && fip->field_tag != TIFFTAG_HALFTONEHINTS - && fip->field_tag != TIFFTAG_YCBCRSUBSAMPLING - && fip->field_tag != TIFFTAG_DOTRANGE) { + || tv->count > 1) { *va_arg(ap, void **) = tv->value; ret_val = 1; } else { - int j; char *val = (char *)tv->value; - for (j = 0; j < tv->count; - j++, val += _TIFFDataSize(tv->info->field_type)) { + assert( tv->count == 1 ); switch (fip->field_type) { case TIFF_BYTE: case TIFF_UNDEFINED: @@ -936,7 +934,6 @@ ret_val = 0; break; } - } } } break; diff -Naur tiff-3.9.7.orig/libtiff/tif_print.c tiff-3.9.7/libtiff/tif_print.c --- tiff-3.9.7.orig/libtiff/tif_print.c 2010-07-08 12:17:59.000000000 -0400 +++ tiff-3.9.7/libtiff/tif_print.c 2012-12-13 13:42:12.773478278 -0500 @@ -112,16 +112,22 @@ } static int -_TIFFPrettyPrintField(TIFF* tif, FILE* fd, ttag_t tag, +_TIFFPrettyPrintField(TIFF* tif, const TIFFFieldInfo *fip, FILE* fd, ttag_t tag, uint32 value_count, void *raw_data) { TIFFDirectory *td = &tif->tif_dir; + /* do not try to pretty print auto-defined fields */ + if (strncmp(fip->field_name,"Tag ", 4) == 0) { + return 0; + } + switch (tag) { case TIFFTAG_INKSET: - fprintf(fd, " Ink Set: "); - switch (*((uint16*)raw_data)) { + if (value_count == 2 && fip->field_type == TIFF_SHORT) { + fprintf(fd, " Ink Set: "); + switch (*((uint16*)raw_data)) { case INKSET_CMYK: fprintf(fd, "CMYK\n"); break; @@ -130,11 +136,18 @@ *((uint16*)raw_data), *((uint16*)raw_data)); break; + } + return 1; } - return 1; + return 0; + case TIFFTAG_WHITEPOINT: - fprintf(fd, " White Point: %g-%g\n", - ((float *)raw_data)[0], ((float *)raw_data)[1]); return 1; + if (value_count == 2 && fip->field_type == TIFF_RATIONAL) { + fprintf(fd, " White Point: %g-%g\n", + ((float *)raw_data)[0], ((float *)raw_data)[1]); return 1; + } + return 0; + case TIFFTAG_REFERENCEBLACKWHITE: { uint16 i; @@ -174,10 +187,13 @@ (unsigned long) value_count); return 1; case TIFFTAG_STONITS: - fprintf(fd, - " Sample to Nits conversion factor: %.4e\n", - *((double*)raw_data)); - return 1; + if (value_count == 1 && fip->field_type == TIFF_DOUBLE) { + fprintf(fd, + " Sample to Nits conversion factor: %.4e\n", + *((double*)raw_data)); + return 1; + } + return 0; } return 0; @@ -524,44 +540,28 @@ value_count = td->td_samplesperpixel; else value_count = fip->field_readcount; - if ((fip->field_type == TIFF_ASCII + if (fip->field_tag == TIFFTAG_DOTRANGE + && strcmp(fip->field_name,"DotRange") == 0) { + /* TODO: This is an evil exception and should not have been + handled this way ... likely best if we move it into + the directory structure with an explicit field in + libtiff 4.1 and assign it a FIELD_ value */ + static uint16 dotrange[2]; + raw_data = dotrange; + TIFFGetField(tif, tag, dotrange+0, dotrange+1); + } else if (fip->field_type == TIFF_ASCII || fip->field_readcount == TIFF_VARIABLE || fip->field_readcount == TIFF_VARIABLE2 || fip->field_readcount == TIFF_SPP - || value_count > 1) - && fip->field_tag != TIFFTAG_PAGENUMBER - && fip->field_tag != TIFFTAG_HALFTONEHINTS - && fip->field_tag != TIFFTAG_YCBCRSUBSAMPLING - && fip->field_tag != TIFFTAG_DOTRANGE) { + || value_count > 1) { if(TIFFGetField(tif, tag, &raw_data) != 1) continue; - } else if (fip->field_tag != TIFFTAG_PAGENUMBER - && fip->field_tag != TIFFTAG_HALFTONEHINTS - && fip->field_tag != TIFFTAG_YCBCRSUBSAMPLING - && fip->field_tag != TIFFTAG_DOTRANGE) { - raw_data = _TIFFmalloc( - _TIFFDataSize(fip->field_type) - * value_count); - mem_alloc = 1; - if(TIFFGetField(tif, tag, raw_data) != 1) { - _TIFFfree(raw_data); - continue; - } } else { - /* - * XXX: Should be fixed and removed, see the - * notes related to TIFFTAG_PAGENUMBER, - * TIFFTAG_HALFTONEHINTS, - * TIFFTAG_YCBCRSUBSAMPLING and - * TIFFTAG_DOTRANGE tags in tif_dir.c. */ - char *tmp; raw_data = _TIFFmalloc( _TIFFDataSize(fip->field_type) * value_count); - tmp = raw_data; mem_alloc = 1; - if(TIFFGetField(tif, tag, tmp, - tmp + _TIFFDataSize(fip->field_type)) != 1) { + if(TIFFGetField(tif, tag, raw_data) != 1) { _TIFFfree(raw_data); continue; } @@ -574,7 +574,7 @@ * _TIFFPrettyPrintField() fall down and print it as any other * tag. */ - if (_TIFFPrettyPrintField(tif, fd, tag, value_count, raw_data)) { + if (_TIFFPrettyPrintField(tif, fip, fd, tag, value_count, raw_data)) { if(mem_alloc) _TIFFfree(raw_data); continue; Teach "tiffinfo -D" to not try to print image data inside an EXIF subdirectory, because there isn't any. Back-patched from an upstream 4.0.2 fix. This is not a security issue in itself (it crashes, but with a simple NULL pointer dereference). However, our test case for CVE-2012-5581 tickles this bug, so it seems easier to fix this than make a new test case. diff -Naur tiff-3.9.4.orig/tools/tiffinfo.c tiff-3.9.4/tools/tiffinfo.c --- tiff-3.9.4.orig/tools/tiffinfo.c 2010-06-08 14:50:44.000000000 -0400 +++ tiff-3.9.4/tools/tiffinfo.c 2012-12-11 16:33:17.062228558 -0500 @@ -49,7 +49,7 @@ int stoponerr = 1; /* stop on first read error */ static void usage(void); -static void tiffinfo(TIFF*, uint16, long); +static void tiffinfo(TIFF*, uint16, long, int); int main(int argc, char* argv[]) @@ -124,19 +124,20 @@ if (tif != NULL) { if (dirnum != -1) { if (TIFFSetDirectory(tif, (tdir_t) dirnum)) - tiffinfo(tif, order, flags); + tiffinfo(tif, order, flags, 1); } else if (diroff != 0) { if (TIFFSetSubDirectory(tif, diroff)) - tiffinfo(tif, order, flags); + tiffinfo(tif, order, flags, 1); } else { do { uint32 offset; - tiffinfo(tif, order, flags); + tiffinfo(tif, order, flags, 1); if (TIFFGetField(tif, TIFFTAG_EXIFIFD, &offset)) { - if (TIFFReadEXIFDirectory(tif, offset)) - tiffinfo(tif, order, flags); + if (TIFFReadEXIFDirectory(tif, offset)) { + tiffinfo(tif, order, flags, 0); + } } } while (TIFFReadDirectory(tif)); } @@ -426,10 +427,10 @@ } static void -tiffinfo(TIFF* tif, uint16 order, long flags) +tiffinfo(TIFF* tif, uint16 order, long flags, int is_image) { TIFFPrintDirectory(tif, stdout, flags); - if (!readdata) + if (!readdata || !is_image) return; if (rawdata) { if (order) { Make TIFFPrintDirectory cope with both TIFF_VARIABLE and TIFF_VARIABLE2 conventions for field_passcount fields, ie, either 16- or 32-bit counts. This patch is taken from upstream commits dated 2012-05-23 ("fix crash with odd 16bit count types for some custom fields") and 2012-12-12 ("Fix TIFF_VARIABLE/TIFF_VARIABLE2 confusion in TIFFPrintDirectory"). This doesn't qualify as a security issue in itself, mainly because TIFFPrintDirectory is unlikely to be used in any security-exposed scenarios; but we need to fix it so that our test case for CVE-2012-5581 works on all platforms. diff -Naur tiff-3.9.4.orig/libtiff/tif_print.c tiff-3.9.4/libtiff/tif_print.c --- tiff-3.9.4.orig/libtiff/tif_print.c 2010-06-08 14:50:42.000000000 -0400 +++ tiff-3.9.4/libtiff/tif_print.c 2012-12-13 12:17:33.726765771 -0500 @@ -518,8 +518,19 @@ continue; if(fip->field_passcount) { - if(TIFFGetField(tif, tag, &value_count, &raw_data) != 1) + if (fip->field_readcount == TIFF_VARIABLE2 ) { + if(TIFFGetField(tif, tag, &value_count, &raw_data) != 1) + continue; + } else if (fip->field_readcount == TIFF_VARIABLE ) { + uint16 small_value_count; + if(TIFFGetField(tif, tag, &small_value_count, &raw_data) != 1) + continue; + value_count = small_value_count; + } else { + assert (fip->field_readcount == TIFF_VARIABLE + || fip->field_readcount == TIFF_VARIABLE2); continue; + } } else { if (fip->field_readcount == TIFF_VARIABLE || fip->field_readcount == TIFF_VARIABLE2)