Merge c641116 into c65941c

Exiv2 · Aug 1, 2021 · 141122b · 141122b
2 parents c65941c + c641116
commit 141122b
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 14 deletions.
diff --git a/src/pngimage.cpp b/src/pngimage.cpp
@@ -317,28 +317,29 @@ namespace Exiv2 {
 
                 if( bDump ) {
                     DataBuf   dataBuf;
-                    auto data = new byte[dataOffset + 1];
-                    data[dataOffset] = 0;
-                    bufRead = io_->read(data,dataOffset);
+                    enforce(static_cast<uint64_t>(dataOffset) < static_cast<unsigned long>(std::numeric_limits<long>::max()), kerFailedToReadImageData);
+                    DataBuf data(static_cast<long>(dataOffset) + 1);
+                    data.pData_[dataOffset] = 0;
+                    bufRead = io_->read(data.pData_, static_cast<long>(dataOffset));
                     enforce(bufRead == static_cast<long>(dataOffset), kerFailedToReadImageData);
                     io_->seek(restore, BasicIo::beg);
-                    uint32_t name_l = static_cast<uint32_t>(std::strlen(reinterpret_cast<const char*>(data))) +
-                                      1;  // leading string length
-                    enforce(name_l <= dataOffset, kerCorruptedMetadata);
+                    size_t name_l = std::strlen(reinterpret_cast<const char*>(data.pData_)) +
+                                    1; // leading string length
+                    enforce(name_l < dataOffset, kerCorruptedMetadata);
 
-                    uint32_t  start  = name_l;
+                    uint32_t  start  = static_cast<uint32_t>(name_l);
                     bool      bLF    = false;
 
                     // decode the chunk
                     bool bGood = false;
                     if ( tEXt ) {
-                        bGood = tEXtToDataBuf(data+name_l,dataOffset-name_l,dataBuf);
+                        bGood = tEXtToDataBuf(data.pData_ + name_l, static_cast<unsigned long>(dataOffset - name_l), dataBuf);
                     }
                     if ( zTXt || iCCP ) {
-                        bGood = zlibToDataBuf(data+name_l+1,dataOffset-name_l-1,dataBuf); // +1 = 'compressed' flag
+                        bGood = zlibToDataBuf(data.pData_ + name_l + 1, static_cast<unsigned long>(dataOffset - name_l - 1), dataBuf); // +1 = 'compressed' flag
                     }
                     if ( iTXt ) {
-                        bGood = (start+3) < dataOffset ;    // good if not a nul chunk
+                        bGood = (3 <= dataOffset) && (start < dataOffset-3); // good if not a nul chunk
                     }
                     if ( eXIf ) {
                         bGood = true ;// eXIf requires no pre-processing)
@@ -347,8 +348,8 @@ namespace Exiv2 {
                     // format is content dependent
                     if ( bGood ) {
                         if ( bXMP ) {
-                            while (start < dataOffset && !data[start]) start++; // skip leading nul bytes
-                            out <<  data+start;             // output the xmp
+                            while (start < dataOffset && !data.pData_[start]) start++; // skip leading nul bytes
+                            out <<  data.pData_ + start;             // output the xmp
                         }
 
                         if ( bExif || bIptc ) {
@@ -389,13 +390,12 @@ namespace Exiv2 {
                         }
                         if ( eXIf && option == kpsRecursive ) {
                             // create memio object with the data, then print the structure
-                            BasicIo::UniquePtr p = BasicIo::UniquePtr(new MemIo(data,dataOffset));
+                            BasicIo::UniquePtr p = BasicIo::UniquePtr(new MemIo(data.pData_, dataOffset));
                             printTiffStructure(*p,out,option,depth);
                         }
 
                         if ( bLF ) out << std::endl;
                     }
-                    delete[] data;
                 }
                 io_->seek(dataOffset+4, BasicIo::cur);// jump past checksum
                 if (io_->error()) throw Error(kerFailedToReadImageData);

diff --git a/test/data/issue_1817_poc.png b/test/data/issue_1817_poc.png
diff --git a/tests/bugfixes/github/test_issue_1817.py b/tests/bugfixes/github/test_issue_1817.py
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, path
+
+class MemoryLeakInPngImagePrintStructure(metaclass=CaseMeta):
+    """
+    Regression test for the bug described in:
+    https://github.com/Exiv2/exiv2/issues/1817
+
+    Note: the test only fails in an ASAN build.
+    """
+    url = "https://github.com/Exiv2/exiv2/issues/1817"
+
+    filename = path("$data_path/issue_1817_poc.png")
+    commands = ["$exiv2 -pS $filename"]
+    stdout = ["""STRUCTURE OF PNG FILE: $filename
+ address | chunk |  length | data                           | checksum
+       8 | eXIf  |       0 |                                | 0x00000000
+"""]
+    stderr = ["""$exiv2_exception_message $filename:
+$kerCorruptedMetadata
+"""]
+    retval = [1]