From f7fc905cff18c7b883712bbfaffabd5d187bc787 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Wed, 21 Feb 2018 06:46:27 -0500 Subject: [PATCH] Fix heap buffer overflows in GetFullCropDimension in tiff_parser.cc Author: timurrrr@google.com Bug: 73646839 Bug: 62307613 Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=6347 --- src/tiff_parser.cc | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/tiff_parser.cc b/src/tiff_parser.cc index 24368e0..6bf3bb4 100644 --- a/src/tiff_parser.cc +++ b/src/tiff_parser.cc @@ -596,23 +596,41 @@ bool GetFullDimension32(const TiffDirectory& tiff_directory, bool GetFullCropDimension(const tiff_directory::TiffDirectory& tiff_directory, std::uint32_t* width, std::uint32_t* height) { - if (tiff_directory.Has(kExifTagDefaultCropSize)) { - std::vector crop(2); - std::vector crop_rational(2); - if (tiff_directory.Get(kExifTagDefaultCropSize, &crop)) { + if (!tiff_directory.Has(kExifTagDefaultCropSize)) { + // This doesn't look right to return true here, as we have not written + // anything to *width and *height. However, changing the return value here + // causes a whole bunch of tests to fail. + // TODO(timurrrr): Return false and fix the tests. + // In fact, this whole if() seems to be not needed, + // as tiff_directory(kExifTagDefaultCropSize) will return false below. + return true; + } + + std::vector crop(2); + if (tiff_directory.Get(kExifTagDefaultCropSize, &crop)) { + if (crop.size() == 2 && crop[0] > 0 && crop[1] > 0) { *width = crop[0]; *height = crop[1]; - } else if (tiff_directory.Get(kExifTagDefaultCropSize, &crop_rational) && - crop_rational[0].denominator != 0 && - crop_rational[1].denominator != 0) { - *width = crop_rational[0].numerator / crop_rational[0].denominator; - *height = crop_rational[1].numerator / crop_rational[1].denominator; + return true; } else { return false; } } - return true; + std::vector crop_rational(2); + if (tiff_directory.Get(kExifTagDefaultCropSize, &crop_rational)) { + if (crop_rational.size() == 2 && crop_rational[0].numerator > 0 && + crop_rational[0].denominator > 0 && crop_rational[1].numerator > 0 && + crop_rational[1].denominator > 0) { + *width = crop_rational[0].numerator / crop_rational[0].denominator; + *height = crop_rational[1].numerator / crop_rational[1].denominator; + return true; + } else { + return false; + } + } + + return false; } TiffParser::TiffParser(StreamInterface* stream) : stream_(stream) {}