From 47b8f070dc11308e0bef3a157f6c70fbcad4093a Mon Sep 17 00:00:00 2001 From: Nicolas Pena Date: Wed, 29 Mar 2017 15:00:08 -0400 Subject: Do more checks before big allocs in TIFFReadDirEntryArray MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL fixes the only caller to TIFFReadDirEntryData with potentially large size so that we avoid big mallocs when we know we will fail. It does this as follows: - Avoid the unnecessary computations if datasize is very small. We don't want to be slower in this case. - If !isMapped(tif), we will Seek and Read. Check that ending position is reachable. In the other case, do a simple check for out of bounds. Bug: chromium:681311 Change-Id: Ia172d8b4d401753b7c8d5455dc1ada5335f6fa6b Reviewed-on: https://pdfium-review.googlesource.com/3253 Commit-Queue: Nicolás Peña Reviewed-by: Lei Zhang --- .../libtiff/0019-oom-TIFFReadDirEntryArray.patch | 81 ++++++++++++++++++++++ third_party/libtiff/README.pdfium | 1 + third_party/libtiff/tif_dirread.c | 65 +++++++++-------- 3 files changed, 114 insertions(+), 33 deletions(-) create mode 100644 third_party/libtiff/0019-oom-TIFFReadDirEntryArray.patch (limited to 'third_party/libtiff') diff --git a/third_party/libtiff/0019-oom-TIFFReadDirEntryArray.patch b/third_party/libtiff/0019-oom-TIFFReadDirEntryArray.patch new file mode 100644 index 0000000000..1144e06ef4 --- /dev/null +++ b/third_party/libtiff/0019-oom-TIFFReadDirEntryArray.patch @@ -0,0 +1,81 @@ +diff --git a/third_party/libtiff/tif_dirread.c b/third_party/libtiff/tif_dirread.c +index 7dd944483..d50b39a80 100644 +--- a/third_party/libtiff/tif_dirread.c ++++ b/third_party/libtiff/tif_dirread.c +@@ -790,44 +790,43 @@ static enum TIFFReadDirEntryErr TIFFReadDirEntryArray(TIFF* tif, TIFFDirEntry* d + *count=(uint32)direntry->tdir_count; + datasize=(*count)*typesize; + assert((tmsize_t)datasize>0); +- data=_TIFFCheckMalloc(tif, *count, typesize, "ReadDirEntryArray"); +- if (data==0) +- return(TIFFReadDirEntryErrAlloc); ++ const uint32 small_alloc_threshold=(tif->tif_flags&TIFF_BIGTIFF)? 8 : 4; ++ if (datasize <= small_alloc_threshold) ++ { ++ data=_TIFFCheckMalloc(tif, *count, typesize, "ReadDirEntryArray"); ++ if (data==0) ++ return(TIFFReadDirEntryErrAlloc); ++ _TIFFmemcpy(data,&direntry->tdir_offset,datasize); ++ *value=data; ++ return(TIFFReadDirEntryErrOk); ++ } ++ uint64 offset; + if (!(tif->tif_flags&TIFF_BIGTIFF)) + { +- if (datasize<=4) +- _TIFFmemcpy(data,&direntry->tdir_offset,datasize); +- else +- { +- enum TIFFReadDirEntryErr err; +- uint32 offset = direntry->tdir_offset.toff_long; +- if (tif->tif_flags&TIFF_SWAB) +- TIFFSwabLong(&offset); +- err=TIFFReadDirEntryData(tif,(uint64)offset,(tmsize_t)datasize,data); +- if (err!=TIFFReadDirEntryErrOk) +- { +- _TIFFfree(data); +- return(err); +- } +- } ++ uint32 small_offset=direntry->tdir_offset.toff_long; ++ if (tif->tif_flags&TIFF_SWAB) ++ TIFFSwabLong(&small_offset); ++ offset=(uint64)small_offset; + } + else + { +- if (datasize<=8) +- _TIFFmemcpy(data,&direntry->tdir_offset,datasize); +- else +- { +- enum TIFFReadDirEntryErr err; +- uint64 offset = direntry->tdir_offset.toff_long8; +- if (tif->tif_flags&TIFF_SWAB) +- TIFFSwabLong8(&offset); +- err=TIFFReadDirEntryData(tif,offset,(tmsize_t)datasize,data); +- if (err!=TIFFReadDirEntryErrOk) +- { +- _TIFFfree(data); +- return(err); +- } +- } ++ offset = direntry->tdir_offset.toff_long8; ++ if (tif->tif_flags&TIFF_SWAB) ++ TIFFSwabLong8(&offset); ++ } ++ if ((uint64)(-1) - offset < datasize) ++ return(TIFFReadDirEntryErrIo); ++ const uint64 size=isMapped(tif)? (uint64)tif->tif_size : TIFFGetFileSize(tif); ++ if (offset + datasize > size) ++ return(TIFFReadDirEntryErrIo); ++ data=_TIFFCheckMalloc(tif, *count, typesize, "ReadDirEntryArray"); ++ if (data==0) ++ return(TIFFReadDirEntryErrAlloc); ++ enum TIFFReadDirEntryErr err=TIFFReadDirEntryData(tif,offset,(tmsize_t)datasize,data); ++ if (err!=TIFFReadDirEntryErrOk) ++ { ++ _TIFFfree(data); ++ return(err); + } + *value=data; + return(TIFFReadDirEntryErrOk); diff --git a/third_party/libtiff/README.pdfium b/third_party/libtiff/README.pdfium index 0b6b800604..b82bb7947a 100644 --- a/third_party/libtiff/README.pdfium +++ b/third_party/libtiff/README.pdfium @@ -23,3 +23,4 @@ Local Modifications: 0015-fix-leaks-in-tif_ojpeg.patch: fix direct leaks in tif_ojpeg.c methods 0017-safe_skews_in_gtTileContig.patch: return error if to/from skews overflow from int32. 0018-fix-leak-in-PredictorSetupDecode.patch: call tif->tif_cleanup if the setup fails. +0019-oom-TIFFReadDirEntryArray.patch: Try to avoid out-of-memory in tif_dirread.c. diff --git a/third_party/libtiff/tif_dirread.c b/third_party/libtiff/tif_dirread.c index 7dd9444834..d50b39a809 100644 --- a/third_party/libtiff/tif_dirread.c +++ b/third_party/libtiff/tif_dirread.c @@ -790,44 +790,43 @@ static enum TIFFReadDirEntryErr TIFFReadDirEntryArray(TIFF* tif, TIFFDirEntry* d *count=(uint32)direntry->tdir_count; datasize=(*count)*typesize; assert((tmsize_t)datasize>0); - data=_TIFFCheckMalloc(tif, *count, typesize, "ReadDirEntryArray"); - if (data==0) - return(TIFFReadDirEntryErrAlloc); + const uint32 small_alloc_threshold=(tif->tif_flags&TIFF_BIGTIFF)? 8 : 4; + if (datasize <= small_alloc_threshold) + { + data=_TIFFCheckMalloc(tif, *count, typesize, "ReadDirEntryArray"); + if (data==0) + return(TIFFReadDirEntryErrAlloc); + _TIFFmemcpy(data,&direntry->tdir_offset,datasize); + *value=data; + return(TIFFReadDirEntryErrOk); + } + uint64 offset; if (!(tif->tif_flags&TIFF_BIGTIFF)) { - if (datasize<=4) - _TIFFmemcpy(data,&direntry->tdir_offset,datasize); - else - { - enum TIFFReadDirEntryErr err; - uint32 offset = direntry->tdir_offset.toff_long; - if (tif->tif_flags&TIFF_SWAB) - TIFFSwabLong(&offset); - err=TIFFReadDirEntryData(tif,(uint64)offset,(tmsize_t)datasize,data); - if (err!=TIFFReadDirEntryErrOk) - { - _TIFFfree(data); - return(err); - } - } + uint32 small_offset=direntry->tdir_offset.toff_long; + if (tif->tif_flags&TIFF_SWAB) + TIFFSwabLong(&small_offset); + offset=(uint64)small_offset; } else { - if (datasize<=8) - _TIFFmemcpy(data,&direntry->tdir_offset,datasize); - else - { - enum TIFFReadDirEntryErr err; - uint64 offset = direntry->tdir_offset.toff_long8; - if (tif->tif_flags&TIFF_SWAB) - TIFFSwabLong8(&offset); - err=TIFFReadDirEntryData(tif,offset,(tmsize_t)datasize,data); - if (err!=TIFFReadDirEntryErrOk) - { - _TIFFfree(data); - return(err); - } - } + offset = direntry->tdir_offset.toff_long8; + if (tif->tif_flags&TIFF_SWAB) + TIFFSwabLong8(&offset); + } + if ((uint64)(-1) - offset < datasize) + return(TIFFReadDirEntryErrIo); + const uint64 size=isMapped(tif)? (uint64)tif->tif_size : TIFFGetFileSize(tif); + if (offset + datasize > size) + return(TIFFReadDirEntryErrIo); + data=_TIFFCheckMalloc(tif, *count, typesize, "ReadDirEntryArray"); + if (data==0) + return(TIFFReadDirEntryErrAlloc); + enum TIFFReadDirEntryErr err=TIFFReadDirEntryData(tif,offset,(tmsize_t)datasize,data); + if (err!=TIFFReadDirEntryErrOk) + { + _TIFFfree(data); + return(err); } *value=data; return(TIFFReadDirEntryErrOk); -- cgit v1.2.3