From f42765c206cc2e4a77f5e3c107a9a258d981c98e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Mon, 16 Jan 2017 18:13:13 +0100 Subject: [PATCH 2/5] Fix CVE-2016-9535 --- libtiff/tif_fax3.c | 3 +- libtiff/tif_lzw.c | 9 +- libtiff/tif_predict.c | 285 +++++++++++++++++++++++++++++++++----------------- libtiff/tif_predict.h | 6 +- 4 files changed, 203 insertions(+), 100 deletions(-) diff --git a/libtiff/tif_fax3.c b/libtiff/tif_fax3.c index 2b2dccd..777ef8f 100644 --- a/libtiff/tif_fax3.c +++ b/libtiff/tif_fax3.c @@ -442,8 +442,9 @@ _TIFFFax3fillruns(unsigned char* buf, uint32* runs, uint32* erun, uint32 lastx) FILL(n, cp); run &= 7; } + /* Explicit 0xff masking to make icc -check=conversions happy */ if (run) - cp[0] |= 0xff00 >> run; + cp[0] = (unsigned char)((cp[0] | (0xff00 >> run))&0xff); } else cp[0] |= _fillmasks[run]>>bx; x += runs[1]; diff --git a/libtiff/tif_lzw.c b/libtiff/tif_lzw.c index fd9c7a0..d7fbe74 100644 --- a/libtiff/tif_lzw.c +++ b/libtiff/tif_lzw.c @@ -830,13 +830,15 @@ LZWPreEncode(TIFF* tif, uint16 s) } else \ rat = (incount<<8) / outcount; \ } + +/* Explicit 0xff masking to make icc -check=conversions happy */ #define PutNextCode(op, c) { \ nextdata = (nextdata << nbits) | c; \ nextbits += nbits; \ - *op++ = (unsigned char)(nextdata >> (nextbits-8)); \ + *op++ = (unsigned char)((nextdata >> (nextbits-8))&0xff); \ nextbits -= 8; \ if (nextbits >= 8) { \ - *op++ = (unsigned char)(nextdata >> (nextbits-8)); \ + *op++ = (unsigned char)((nextdata >> (nextbits-8))&0xff); \ nextbits -= 8; \ } \ outcount += nbits; \ @@ -1042,8 +1044,9 @@ LZWPostEncode(TIFF* tif) sp->enc_oldcode = (hcode_t) -1; } PutNextCode(op, CODE_EOI); + /* Explicit 0xff masking to make icc -check=conversions happy */ if (nextbits > 0) - *op++ = (unsigned char)(nextdata << (8-nextbits)); + *op++ = (unsigned char)((nextdata << (8-nextbits))&0xff); tif->tif_rawcc = (tmsize_t)(op - tif->tif_rawdata); return (1); } diff --git a/libtiff/tif_predict.c b/libtiff/tif_predict.c index f93c664..39d764f 100644 --- a/libtiff/tif_predict.c +++ b/libtiff/tif_predict.c @@ -34,16 +34,18 @@ #define PredictorState(tif) ((TIFFPredictorState*) (tif)->tif_data) -static void horAcc8(TIFF* tif, uint8* cp0, tmsize_t cc); -static void horAcc16(TIFF* tif, uint8* cp0, tmsize_t cc); -static void horAcc32(TIFF* tif, uint8* cp0, tmsize_t cc); -static void swabHorAcc16(TIFF* tif, uint8* cp0, tmsize_t cc); -static void swabHorAcc32(TIFF* tif, uint8* cp0, tmsize_t cc); -static void horDiff8(TIFF* tif, uint8* cp0, tmsize_t cc); -static void horDiff16(TIFF* tif, uint8* cp0, tmsize_t cc); -static void horDiff32(TIFF* tif, uint8* cp0, tmsize_t cc); -static void fpAcc(TIFF* tif, uint8* cp0, tmsize_t cc); -static void fpDiff(TIFF* tif, uint8* cp0, tmsize_t cc); +static int horAcc8(TIFF* tif, uint8* cp0, tmsize_t cc); +static int horAcc16(TIFF* tif, uint8* cp0, tmsize_t cc); +static int horAcc32(TIFF* tif, uint8* cp0, tmsize_t cc); +static int swabHorAcc16(TIFF* tif, uint8* cp0, tmsize_t cc); +static int swabHorAcc32(TIFF* tif, uint8* cp0, tmsize_t cc); +static int horDiff8(TIFF* tif, uint8* cp0, tmsize_t cc); +static int horDiff16(TIFF* tif, uint8* cp0, tmsize_t cc); +static int horDiff32(TIFF* tif, uint8* cp0, tmsize_t cc); +static int swabHorDiff16(TIFF* tif, uint8* cp0, tmsize_t cc); +static int swabHorDiff32(TIFF* tif, uint8* cp0, tmsize_t cc); +static int fpAcc(TIFF* tif, uint8* cp0, tmsize_t cc); +static int fpDiff(TIFF* tif, uint8* cp0, tmsize_t cc); static int PredictorDecodeRow(TIFF* tif, uint8* op0, tmsize_t occ0, uint16 s); static int PredictorDecodeTile(TIFF* tif, uint8* op0, tmsize_t occ0, uint16 s); static int PredictorEncodeRow(TIFF* tif, uint8* bp, tmsize_t cc, uint16 s); @@ -207,7 +209,24 @@ PredictorSetupEncode(TIFF* tif) sp->encodetile = tif->tif_encodetile; tif->tif_encodetile = PredictorEncodeTile; } - } + + /* + * If the data is horizontally differenced 16-bit data that + * requires byte-swapping, then it must be byte swapped after + * the differenciation step. We do this with a special-purpose + * routine and override the normal post decoding logic that + * the library setup when the directory was read. + */ + if (tif->tif_flags & TIFF_SWAB) { + if (sp->encodepfunc == horDiff16) { + sp->encodepfunc = swabHorDiff16; + tif->tif_postdecode = _TIFFNoPostDecode; + } else if (sp->encodepfunc == horDiff32) { + sp->encodepfunc = swabHorDiff32; + tif->tif_postdecode = _TIFFNoPostDecode; + } + } + } else if (sp->predictor == 3) { sp->encodepfunc = fpDiff; @@ -239,13 +258,25 @@ PredictorSetupEncode(TIFF* tif) case 0: ; \ } -static void +/* Remarks related to C standard compliance in all below functions : */ +/* - to avoid any undefined behaviour, we only operate on unsigned types */ +/* since the behaviour of "overflows" is defined (wrap over) */ +/* - when storing into the byte stream, we explicitly mask with 0xff so */ +/* as to make icc -check=conversions happy (not necessary by the standard) */ + +static int horAcc8(TIFF* tif, uint8* cp0, tmsize_t cc) { tmsize_t stride = PredictorState(tif)->stride; - char* cp = (char*) cp0; - assert((cc%stride)==0); + unsigned char* cp = (unsigned char*) cp0; + if((cc%stride)!=0) + { + TIFFErrorExt(tif->tif_clientdata, "horAcc8", + "%s", "(cc%stride)!=0"); + return 0; + } + if (cc > stride) { /* * Pipeline the most common cases. @@ -257,9 +288,9 @@ horAcc8(TIFF* tif, uint8* cp0, tmsize_t cc) cc -= 3; cp += 3; while (cc>0) { - cp[0] = (char) (cr += cp[0]); - cp[1] = (char) (cg += cp[1]); - cp[2] = (char) (cb += cp[2]); + cp[0] = (unsigned char) ((cr += cp[0]) & 0xff); + cp[1] = (unsigned char) ((cg += cp[1]) & 0xff); + cp[2] = (unsigned char) ((cb += cp[2]) & 0xff); cc -= 3; cp += 3; } @@ -271,10 +302,10 @@ horAcc8(TIFF* tif, uint8* cp0, tmsize_t cc) cc -= 4; cp += 4; while (cc>0) { - cp[0] = (char) (cr += cp[0]); - cp[1] = (char) (cg += cp[1]); - cp[2] = (char) (cb += cp[2]); - cp[3] = (char) (ca += cp[3]); + cp[0] = (unsigned char) ((cr += cp[0]) & 0xff); + cp[1] = (unsigned char) ((cg += cp[1]) & 0xff); + cp[2] = (unsigned char) ((cb += cp[2]) & 0xff); + cp[3] = (unsigned char) ((ca += cp[3]) & 0xff); cc -= 4; cp += 4; } @@ -282,77 +313,71 @@ horAcc8(TIFF* tif, uint8* cp0, tmsize_t cc) cc -= stride; do { REPEAT4(stride, cp[stride] = - (char) (cp[stride] + *cp); cp++) + (unsigned char) ((cp[stride] + *cp) & 0xff); cp++) cc -= stride; } while (cc>0); } } + return 1; } -static void +static int swabHorAcc16(TIFF* tif, uint8* cp0, tmsize_t cc) { - tmsize_t stride = PredictorState(tif)->stride; uint16* wp = (uint16*) cp0; tmsize_t wc = cc / 2; - assert((cc%(2*stride))==0); - - if (wc > stride) { - TIFFSwabArrayOfShort(wp, wc); - wc -= stride; - do { - REPEAT4(stride, wp[stride] += wp[0]; wp++) - wc -= stride; - } while (wc > 0); - } + TIFFSwabArrayOfShort(wp, wc); + return horAcc16(tif, cp0, cc); } -static void +static int horAcc16(TIFF* tif, uint8* cp0, tmsize_t cc) { tmsize_t stride = PredictorState(tif)->stride; uint16* wp = (uint16*) cp0; tmsize_t wc = cc / 2; - assert((cc%(2*stride))==0); + if((cc%(2*stride))!=0) + { + TIFFErrorExt(tif->tif_clientdata, "horAcc16", + "%s", "cc%(2*stride))!=0"); + return 0; + } if (wc > stride) { wc -= stride; do { - REPEAT4(stride, wp[stride] += wp[0]; wp++) + REPEAT4(stride, wp[stride] = (uint16)(((unsigned int)wp[stride] + (unsigned int)wp[0]) & 0xffff); wp++) wc -= stride; } while (wc > 0); } + return 1; } -static void +static int swabHorAcc32(TIFF* tif, uint8* cp0, tmsize_t cc) { - tmsize_t stride = PredictorState(tif)->stride; uint32* wp = (uint32*) cp0; tmsize_t wc = cc / 4; - assert((cc%(4*stride))==0); - - if (wc > stride) { - TIFFSwabArrayOfLong(wp, wc); - wc -= stride; - do { - REPEAT4(stride, wp[stride] += wp[0]; wp++) - wc -= stride; - } while (wc > 0); - } + TIFFSwabArrayOfLong(wp, wc); + return horAcc32(tif, cp0, cc); } -static void +static int horAcc32(TIFF* tif, uint8* cp0, tmsize_t cc) { tmsize_t stride = PredictorState(tif)->stride; uint32* wp = (uint32*) cp0; tmsize_t wc = cc / 4; - assert((cc%(4*stride))==0); + if((cc%(4*stride))!=0) + { + TIFFErrorExt(tif->tif_clientdata, "horAcc32", + "%s", "cc%(4*stride))!=0"); + return 0; + } if (wc > stride) { wc -= stride; @@ -361,12 +386,13 @@ horAcc32(TIFF* tif, uint8* cp0, tmsize_t cc) wc -= stride; } while (wc > 0); } + return 1; } /* * Floating point predictor accumulation routine. */ -static void +static int fpAcc(TIFF* tif, uint8* cp0, tmsize_t cc) { tmsize_t stride = PredictorState(tif)->stride; @@ -374,15 +400,22 @@ fpAcc(TIFF* tif, uint8* cp0, tmsize_t cc) tmsize_t wc = cc / bps; tmsize_t count = cc; uint8 *cp = (uint8 *) cp0; - uint8 *tmp = (uint8 *)_TIFFmalloc(cc); + uint8 *tmp; - assert((cc%(bps*stride))==0); + if(cc%(bps*stride)!=0) + { + TIFFErrorExt(tif->tif_clientdata, "fpAcc", + "%s", "cc%(bps*stride))!=0"); + return 0; + } + tmp = (uint8 *)_TIFFmalloc(cc); if (!tmp) - return; + return 0; while (count > stride) { - REPEAT4(stride, cp[stride] += cp[0]; cp++) + REPEAT4(stride, cp[stride] = + (unsigned char) ((cp[stride] + cp[0]) & 0xff); cp++) count -= stride; } @@ -400,6 +433,7 @@ fpAcc(TIFF* tif, uint8* cp0, tmsize_t cc) } } _TIFFfree(tmp); + return 1; } /* @@ -415,8 +449,7 @@ PredictorDecodeRow(TIFF* tif, uint8* op0, tmsize_t occ0, uint16 s) assert(sp->decodepfunc != NULL); if ((*sp->decoderow)(tif, op0, occ0, s)) { - (*sp->decodepfunc)(tif, op0, occ0); - return 1; + return (*sp->decodepfunc)(tif, op0, occ0); } else return 0; } @@ -439,10 +472,16 @@ PredictorDecodeTile(TIFF* tif, uint8* op0, tmsize_t occ0, uint16 s) if ((*sp->decodetile)(tif, op0, occ0, s)) { tmsize_t rowsize = sp->rowsize; assert(rowsize > 0); - assert((occ0%rowsize)==0); + if((occ0%rowsize) !=0) + { + TIFFErrorExt(tif->tif_clientdata, "PredictorDecodeTile", + "%s", "occ0%rowsize != 0"); + return 0; + } assert(sp->decodepfunc != NULL); while (occ0 > 0) { - (*sp->decodepfunc)(tif, op0, rowsize); + if( !(*sp->decodepfunc)(tif, op0, rowsize) ) + return 0; occ0 -= rowsize; op0 += rowsize; } @@ -451,14 +490,19 @@ PredictorDecodeTile(TIFF* tif, uint8* op0, tmsize_t occ0, uint16 s) return 0; } -static void +static int horDiff8(TIFF* tif, uint8* cp0, tmsize_t cc) { TIFFPredictorState* sp = PredictorState(tif); tmsize_t stride = sp->stride; - char* cp = (char*) cp0; + unsigned char* cp = (unsigned char*) cp0; - assert((cc%stride)==0); + if((cc%stride)!=0) + { + TIFFErrorExt(tif->tif_clientdata, "horDiff8", + "%s", "(cc%stride)!=0"); + return 0; + } if (cc > stride) { cc -= stride; @@ -466,67 +510,92 @@ horDiff8(TIFF* tif, uint8* cp0, tmsize_t cc) * Pipeline the most common cases. */ if (stride == 3) { - int r1, g1, b1; - int r2 = cp[0]; - int g2 = cp[1]; - int b2 = cp[2]; + unsigned int r1, g1, b1; + unsigned int r2 = cp[0]; + unsigned int g2 = cp[1]; + unsigned int b2 = cp[2]; do { - r1 = cp[3]; cp[3] = r1-r2; r2 = r1; - g1 = cp[4]; cp[4] = g1-g2; g2 = g1; - b1 = cp[5]; cp[5] = b1-b2; b2 = b1; + r1 = cp[3]; cp[3] = (unsigned char)((r1-r2)&0xff); r2 = r1; + g1 = cp[4]; cp[4] = (unsigned char)((g1-g2)&0xff); g2 = g1; + b1 = cp[5]; cp[5] = (unsigned char)((b1-b2)&0xff); b2 = b1; cp += 3; } while ((cc -= 3) > 0); } else if (stride == 4) { - int r1, g1, b1, a1; - int r2 = cp[0]; - int g2 = cp[1]; - int b2 = cp[2]; - int a2 = cp[3]; + unsigned int r1, g1, b1, a1; + unsigned int r2 = cp[0]; + unsigned int g2 = cp[1]; + unsigned int b2 = cp[2]; + unsigned int a2 = cp[3]; do { - r1 = cp[4]; cp[4] = r1-r2; r2 = r1; - g1 = cp[5]; cp[5] = g1-g2; g2 = g1; - b1 = cp[6]; cp[6] = b1-b2; b2 = b1; - a1 = cp[7]; cp[7] = a1-a2; a2 = a1; + r1 = cp[4]; cp[4] = (unsigned char)((r1-r2)&0xff); r2 = r1; + g1 = cp[5]; cp[5] = (unsigned char)((g1-g2)&0xff); g2 = g1; + b1 = cp[6]; cp[6] = (unsigned char)((b1-b2)&0xff); b2 = b1; + a1 = cp[7]; cp[7] = (unsigned char)((a1-a2)&0xff); a2 = a1; cp += 4; } while ((cc -= 4) > 0); } else { cp += cc - 1; do { - REPEAT4(stride, cp[stride] -= cp[0]; cp--) + REPEAT4(stride, cp[stride] = (unsigned char)((cp[stride] - cp[0])&0xff); cp--) } while ((cc -= stride) > 0); } } + return 1; } -static void +static int horDiff16(TIFF* tif, uint8* cp0, tmsize_t cc) { TIFFPredictorState* sp = PredictorState(tif); tmsize_t stride = sp->stride; - int16 *wp = (int16*) cp0; + uint16 *wp = (uint16*) cp0; tmsize_t wc = cc/2; - assert((cc%(2*stride))==0); + if((cc%(2*stride))!=0) + { + TIFFErrorExt(tif->tif_clientdata, "horDiff8", + "%s", "(cc%(2*stride))!=0"); + return 0; + } if (wc > stride) { wc -= stride; wp += wc - 1; do { - REPEAT4(stride, wp[stride] -= wp[0]; wp--) + REPEAT4(stride, wp[stride] = (uint16)(((unsigned int)wp[stride] - (unsigned int)wp[0]) & 0xffff); wp--) wc -= stride; } while (wc > 0); } + return 1; } -static void +static int +swabHorDiff16(TIFF* tif, uint8* cp0, tmsize_t cc) +{ + uint16* wp = (uint16*) cp0; + tmsize_t wc = cc / 2; + + if( !horDiff16(tif, cp0, cc) ) + return 0; + + TIFFSwabArrayOfShort(wp, wc); + return 1; +} + +static int horDiff32(TIFF* tif, uint8* cp0, tmsize_t cc) { TIFFPredictorState* sp = PredictorState(tif); tmsize_t stride = sp->stride; - int32 *wp = (int32*) cp0; + uint32 *wp = (uint32*) cp0; tmsize_t wc = cc/4; - assert((cc%(4*stride))==0); + if((cc%(4*stride))!=0) + { + TIFFErrorExt(tif->tif_clientdata, "horDiff32", + "%s", "(cc%(4*stride))!=0"); + return 0; + } if (wc > stride) { wc -= stride; @@ -536,12 +605,26 @@ horDiff32(TIFF* tif, uint8* cp0, tmsize_t cc) wc -= stride; } while (wc > 0); } + return 1; +} + +static int +swabHorDiff32(TIFF* tif, uint8* cp0, tmsize_t cc) +{ + uint32* wp = (uint32*) cp0; + tmsize_t wc = cc / 4; + + if( !horDiff32(tif, cp0, cc) ) + return 0; + + TIFFSwabArrayOfLong(wp, wc); + return 1; } /* * Floating point predictor differencing routine. */ -static void +static int fpDiff(TIFF* tif, uint8* cp0, tmsize_t cc) { tmsize_t stride = PredictorState(tif)->stride; @@ -549,12 +632,18 @@ fpDiff(TIFF* tif, uint8* cp0, tmsize_t cc) tmsize_t wc = cc / bps; tmsize_t count; uint8 *cp = (uint8 *) cp0; - uint8 *tmp = (uint8 *)_TIFFmalloc(cc); + uint8 *tmp; - assert((cc%(bps*stride))==0); + if((cc%(bps*stride))!=0) + { + TIFFErrorExt(tif->tif_clientdata, "fpDiff", + "%s", "(cc%(bps*stride))!=0"); + return 0; + } + tmp = (uint8 *)_TIFFmalloc(cc); if (!tmp) - return; + return 0; _TIFFmemcpy(tmp, cp0, cc); for (count = 0; count < wc; count++) { @@ -573,7 +662,8 @@ fpDiff(TIFF* tif, uint8* cp0, tmsize_t cc) cp = (uint8 *) cp0; cp += cc - stride - 1; for (count = cc; count > stride; count -= stride) - REPEAT4(stride, cp[stride] -= cp[0]; cp--) + REPEAT4(stride, cp[stride] = (unsigned char)((cp[stride] - cp[0])&0xff); cp--) + return 1; } static int @@ -586,7 +676,8 @@ PredictorEncodeRow(TIFF* tif, uint8* bp, tmsize_t cc, uint16 s) assert(sp->encoderow != NULL); /* XXX horizontal differencing alters user's data XXX */ - (*sp->encodepfunc)(tif, bp, cc); + if( !(*sp->encodepfunc)(tif, bp, cc) ) + return 0; return (*sp->encoderow)(tif, bp, cc, s); } @@ -621,7 +712,13 @@ PredictorEncodeTile(TIFF* tif, uint8* bp0, tmsize_t cc0, uint16 s) rowsize = sp->rowsize; assert(rowsize > 0); - assert((cc0%rowsize)==0); + if((cc0%rowsize)!=0) + { + TIFFErrorExt(tif->tif_clientdata, "PredictorEncodeTile", + "%s", "(cc0%rowsize)!=0"); + _TIFFfree( working_copy ); + return 0; + } while (cc > 0) { (*sp->encodepfunc)(tif, bp, rowsize); cc -= rowsize; diff --git a/libtiff/tif_predict.h b/libtiff/tif_predict.h index dc7144c..1585a58 100644 --- a/libtiff/tif_predict.h +++ b/libtiff/tif_predict.h @@ -30,6 +30,8 @@ * ``Library-private'' Support for the Predictor Tag */ +typedef int (*TIFFEncodeDecodeMethod)(TIFF* tif, uint8* buf, tmsize_t size); + /* * Codecs that want to support the Predictor tag must place * this structure first in their private state block so that @@ -43,12 +45,12 @@ typedef struct { TIFFCodeMethod encoderow; /* parent codec encode/decode row */ TIFFCodeMethod encodestrip; /* parent codec encode/decode strip */ TIFFCodeMethod encodetile; /* parent codec encode/decode tile */ - TIFFPostMethod encodepfunc; /* horizontal differencer */ + TIFFEncodeDecodeMethod encodepfunc; /* horizontal differencer */ TIFFCodeMethod decoderow; /* parent codec encode/decode row */ TIFFCodeMethod decodestrip; /* parent codec encode/decode strip */ TIFFCodeMethod decodetile; /* parent codec encode/decode tile */ - TIFFPostMethod decodepfunc; /* horizontal accumulator */ + TIFFEncodeDecodeMethod decodepfunc; /* horizontal accumulator */ TIFFVGetMethod vgetparent; /* super-class method */ TIFFVSetMethod vsetparent; /* super-class method */ -- 2.7.4