From 35b3e596edacd2437c2c5d3dd2b5c9502626163d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= <dan.cermak@cgc-instruments.com>
Date: Fri, 17 Aug 2018 16:41:05 +0200
Subject: [PATCH] Add overflow & overread checks to PngChunk::parseTXTChunk()

This function was creating a lot of new pointers and strings without
properly checking the array bounds. This commit adds several calls
to enforce(), making sure that the pointers stay within bounds.
Strings are now created using the helper function
string_from_unterminated() to prevent overreads in the constructor of
std::string.

This fixes #400
---
 src/pngchunk_int.cpp | 63 ++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/src/pngchunk.cpp b/src/pngchunk.cpp
index e087e47cb..f13594bd8 100644
--- a/src/pngchunk.cpp
+++ b/src/pngchunk.cpp
@@ -34,6 +34,7 @@
 #include "image.hpp"
 #include "error.hpp"
 #include "enforce.hpp"
+#include "safe_op.hpp"
 
 // + standard includes
 #include <sstream>
@@ -55,6 +54,13 @@ EXIV2_RCSID("@(#) $Id: pngchunk.cpp 3777 2015-05-02 11:55:40Z ahuggel $")
 
 #include <zlib.h>     // To uncompress or compress text chunk
 
+/* Added as support of CVE-2018-16336 */
+std::string string_from_unterminated(const char* data, size_t data_length)
+{
+    const size_t StringLength = strnlen(data, data_length);
+
+    return std::string(data, StringLength);
+}
 /*
 
 URLs to find informations about PNG chunks :
@@ -133,6 +135,8 @@ namespace Exiv2 {
 
         if(type == zTXt_Chunk)
         {
+            enforce(data.size_ >= Safe::add(keysize, 2), Exiv2::kerCorruptedMetadata);
+
             // Extract a deflate compressed Latin-1 text chunk
 
             // we get the compression method after the key
@@ -149,11 +153,13 @@ namespace Exiv2 {
             // compressed string after the compression technique spec
             const byte* compressedText      = data.pData_ + keysize + 2;
             unsigned int compressedTextSize = data.size_  - keysize - 2;
+            enforce(compressedTextSize < data.size_, kerCorruptedMetadata);
 
             zlibUncompress(compressedText, compressedTextSize, arr);
         }
         else if(type == tEXt_Chunk)
         {
+            enforce(data.size_ >= Safe::add(keysize, 1), Exiv2::kerCorruptedMetadata);
             // Extract a non-compressed Latin-1 text chunk
 
             // the text comes after the key, but isn't null terminated
@@ -164,7 +170,8 @@ namespace Exiv2 {
         }
         else if(type == iTXt_Chunk)
         {
+            enforce(data.size_ >= Safe::add(keysize, 3), Exiv2::kerCorruptedMetadata);
             const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_], '\0');

             enforce(nullSeparators >= 2, Exiv2::kerCorruptedMetadata);
 
@@ -178,40 +185,44 @@ namespace Exiv2 {
             enforce(compressionMethod == 0x00, Exiv2::kerCorruptedMetadata);
             // language description string after the compression technique spec
-            std::string languageText((const char*)(data.pData_ + keysize + 3));
-            unsigned int languageTextSize = static_cast<unsigned int>(languageText.size());
+            const size_t languageTextMaxSize = data.size_ - keysize - 3;
+            std::string languageText =
+                string_from_unterminated((const char*)(data.pData_ + Safe::add(keysize, 3)), languageTextMaxSize);
+            const unsigned int languageTextSize = static_cast<unsigned int>(languageText.size());
+
+            enforce(data.size_ >= Safe::add(static_cast<unsigned int>(Safe::add(keysize, 4)), languageTextSize),
+                    Exiv2::kerCorruptedMetadata);
             // translated keyword string after the language description
-            std::string translatedKeyText((const char*)(data.pData_ + keysize + 3 + languageTextSize +1));
-            unsigned int translatedKeyTextSize = static_cast<unsigned int>(translatedKeyText.size());
+            std::string translatedKeyText =
+                string_from_unterminated((const char*)(data.pData_ + keysize + 3 + languageTextSize + 1),
+                                         data.size_ - (keysize + 3 + languageTextSize + 1));
+            const unsigned int translatedKeyTextSize = static_cast<unsigned int>(translatedKeyText.size());
 
-            if ( compressionFlag == 0x00 )
-            {
-                // then it's an uncompressed iTXt chunk
-#ifdef DEBUG
-                std::cout << "Exiv2::PngChunk::parseTXTChunk: We found an uncompressed iTXt field\n";
-#endif
+            if ((compressionFlag == 0x00) || (compressionFlag == 0x01 && compressionMethod == 0x00)) {
+                enforce(Safe::add(static_cast<unsigned int>(keysize + 3 + languageTextSize + 1),
+                                  Safe::add(translatedKeyTextSize, 1u)) <= data.size_,
+                        Exiv2::kerCorruptedMetadata);
 
-                // the text comes after the translated keyword, but isn't null terminated
                 const byte* text = data.pData_ + keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1;
-                long textsize    = data.size_ - (keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1);
+                const long textsize = data.size_ - (keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1);
 
-                arr.alloc(textsize);
-                arr = DataBuf(text, textsize);
-            }
-            else if ( compressionFlag == 0x01 && compressionMethod == 0x00 )
-            {
-                // then it's a zlib compressed iTXt chunk
+                if (compressionFlag == 0x00) {
+                    // then it's an uncompressed iTXt chunk
 #ifdef DEBUG
-                std::cout << "Exiv2::PngChunk::parseTXTChunk: We found a zlib compressed iTXt field\n";
+                    std::cout << "Exiv2::PngChunk::parseTXTChunk: We found an uncompressed iTXt field\n";
 #endif
 
-                // the compressed text comes after the translated keyword, but isn't null terminated
-                const byte* compressedText = data.pData_ + keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1;
-                long compressedTextSize    = data.size_ - (keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1);
+                    arr.alloc(textsize);
+                    arr = DataBuf(text, textsize);
+                } else if (compressionFlag == 0x01 && compressionMethod == 0x00) {
+                    // then it's a zlib compressed iTXt chunk
+#ifdef DEBUG
+                    std::cout << "Exiv2::PngChunk::parseTXTChunk: We found a zlib compressed iTXt field\n";
+#endif
 
-                zlibUncompress(compressedText, compressedTextSize, arr);
-            }
-            else
-            {
+                    // the compressed text comes after the translated keyword, but isn't null terminated
+                    zlibUncompress(text, textsize, arr);
+                }
+            } else {
                 // then it isn't zlib compressed and we are sunk
 #ifdef DEBUG
                 std::cerr << "Exiv2::PngChunk::parseTXTChunk: Non-standard iTXt compression method.\n";