From 800527edd2f8ba113123c881cf1c1fbebb62db6b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 10 Dec 2012 18:19:11 +0000 Subject: [PATCH] Improve previous patch for CVE-2012-4564. --- ChangeLog | 7 +++++++ tools/ppm2tiff.c | 50 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index f6251ccf..21ef4242 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2012-12-10 Tom Lane + + * tools/ppm2tiff.c: Improve previous patch for CVE-2012-4564: + check the linebytes calculation too, get the max() calculation + straight, avoid redundant error messages, check for malloc + failure. + 2012-12-10 Tom Lane * libtiff/tif_pixarlog.c: Improve previous patch for CVE-2012-4447 diff --git a/tools/ppm2tiff.c b/tools/ppm2tiff.c index 1a6d7f62..70f74be5 100644 --- a/tools/ppm2tiff.c +++ b/tools/ppm2tiff.c @@ -1,4 +1,4 @@ -/* $Id: ppm2tiff.c,v 1.17 2012-11-02 05:13:24 fwarmerdam Exp $ */ +/* $Id: ppm2tiff.c,v 1.18 2012-12-10 18:19:11 tgl Exp $ */ /* * Copyright (c) 1991-1997 Sam Leffler @@ -72,6 +72,17 @@ BadPPM(char* file) exit(-2); } +static tmsize_t +multiply_ms(tmsize_t m1, tmsize_t m2) +{ + tmsize_t bytes = m1 * m2; + + if (m1 && bytes / m1 != m2) + bytes = 0; + + return bytes; +} + int main(int argc, char* argv[]) { @@ -79,7 +90,7 @@ main(int argc, char* argv[]) uint32 rowsperstrip = (uint32) -1; double resolution = -1; unsigned char *buf = NULL; - tsize_t linebytes = 0; + tmsize_t linebytes = 0; uint16 spp = 1; uint16 bpp = 8; TIFF *out; @@ -222,7 +233,8 @@ main(int argc, char* argv[]) } 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 { @@ -231,23 +243,31 @@ main(int argc, char* argv[]) } 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 { - scanline_size = TIFFScanlineSize(out); - if (scanline_size != 0) - buf = (unsigned char *)_TIFFmalloc(TIFFScanlineSize(out)); - else { - fprintf(stderr, "%s: scanline size overflow\n",infile); - (void) TIFFClose(out); - exit(-2); - } - } + else + 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);