Merge pull request #2428 from kevinbackhouse/fix-issue-2427

Fix memory leak in BmffImage::brotliUncompress

src/bmffimage.cpp

@@ -164,13 +164,30 @@ std::string BmffImage::uuidName(const Exiv2::DataBuf& uuid) {
 }
 
 #ifdef EXV_HAVE_BROTLI
-void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBufSize, DataBuf& arr) {
-  BrotliDecoderState* decoder = NULL;
-  decoder = BrotliDecoderCreateInstance(NULL, NULL, NULL);
-  if (!decoder) {
-    throw Error(ErrorCode::kerMallocFailed);
+
+// Wrapper class for BrotliDecoderState that automatically calls
+// BrotliDecoderDestroyInstance in its destructor.
+class BrotliDecoderWrapper {
+  BrotliDecoderState* decoder_;
+
+ public:
+  BrotliDecoderWrapper() : decoder_(BrotliDecoderCreateInstance(NULL, NULL, NULL)) {
+    if (!decoder_) {
+      throw Error(ErrorCode::kerMallocFailed);
+    }
   }
 
+  ~BrotliDecoderWrapper() {
+    BrotliDecoderDestroyInstance(decoder_);
+  }
+
+  BrotliDecoderState* get() const {
+    return decoder_;
+  }
+};
+
+void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBufSize, DataBuf& arr) {
+  BrotliDecoderWrapper decoder;
   size_t uncompressedLen = compressedBufSize * 2;  // just a starting point
   BrotliDecoderResult result;
   int dos = 0;
@@ -184,7 +201,8 @@ void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBuf
     arr.alloc(uncompressedLen);
     available_out = uncompressedLen - total_out;
     next_out = arr.data() + total_out;
-    result = BrotliDecoderDecompressStream(decoder, &available_in, &next_in, &available_out, &next_out, &total_out);
+    result =
+        BrotliDecoderDecompressStream(decoder.get(), &available_in, &next_in, &available_out, &next_out, &total_out);
     if (result == BROTLI_DECODER_RESULT_SUCCESS) {
       arr.resize(total_out);
     } else if (result == BROTLI_DECODER_RESULT_NEEDS_MORE_OUTPUT) {
@@ -200,12 +218,10 @@ void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBuf
       throw Error(ErrorCode::kerFailedToReadImageData);
     } else {
       // something bad happened
-      throw Error(ErrorCode::kerErrorMessage, BrotliDecoderErrorString(BrotliDecoderGetErrorCode(decoder)));
+      throw Error(ErrorCode::kerErrorMessage, BrotliDecoderErrorString(BrotliDecoderGetErrorCode(decoder.get())));
     }
   } while (result != BROTLI_DECODER_RESULT_SUCCESS);
 
-  BrotliDecoderDestroyInstance(decoder);
-
   if (result != BROTLI_DECODER_RESULT_SUCCESS) {
     throw Error(ErrorCode::kerFailedToReadImageData);
   }

tests/bugfixes/github/test_issue_2427.py

@@ -0,0 +1,13 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, check_no_ASAN_UBSAN_errors
+
+class issue_2427_BmffImage_brotliUncompress_memleak(metaclass=CaseMeta):
+    url      = "https://github.com/Exiv2/exiv2/issues/2427"
+    filename = "$data_path/issue_2427_poc.jpg"
+    commands = ["$exiv2 $filename"]
+    retval   = [1]
+    stderr   = ["""$exiv2_exception_message $filename:
+CL_SPACE
+"""]
+    stdout   = [""]

tests/regression_tests/test_regression_allfiles.py

@@ -66,6 +66,7 @@ def get_valid_files(data_dir):
         "issue_2383_poc.mp4",
         "issue_2393_poc.mp4",
         "issue_2423_poc.mp4",
+        "issue_2427_poc.jpg",
         "2018-01-09-exiv2-crash-001.tiff",
         "cve_2017_1000126_stack-oob-read.webp",
         "exiv2-bug1247.jpg",