From 938f8b6e49ae43b062f76aad968ff76f5f33c965 Mon Sep 17 00:00:00 2001
From: Leon Scroggins III <scroggo@google.com>
Date: Wed, 21 Feb 2018 06:46:27 -0500
Subject: Fix heap buffer overflows in GetFullCropDimension in tiff_parser.cc

Author: timurrrr@google.com
---
 src/tiff_parser.cc | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

(limited to 'src')

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<std::uint32_t> crop(2);
-    std::vector<Rational> 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<std::uint32_t> 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) {
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  std::vector<Rational> 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 true;
+  return false;
 }
 
 TiffParser::TiffParser(StreamInterface* stream) : stream_(stream) {}
-- 
cgit v1.2.3