From: Dirk Lemstra <dirk@git.imagemagick.org>
Date: Thu, 27 Apr 2017 09:50:51 +0000 (+0200)
Subject: Refactored ReadMNGImage to fix memory leak reported in #442
X-Git-Tag: 7.0.5-6~40
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7dea03d2f10279c065dad4be2591f4b160660636;p=imagemagick

Refactored ReadMNGImage to fix memory leak reported in #442
---

diff --git a/coders/png.c b/coders/png.c
index 310a83939..6a6b4c8e6 100644
--- a/coders/png.c
+++ b/coders/png.c
@@ -5268,7 +5268,7 @@ static Image *ReadJNGImage(const ImageInfo *image_info,
 }
 #endif
 
-static Image *ReadMNGImage(const ImageInfo *image_info,
+static Image *ReadOneMNGImage(MngInfo* mng_info, const ImageInfo *image_info,
      ExceptionInfo *exception)
 {
   char
@@ -5295,9 +5295,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
   MagickOffsetType
     offset;
 
-  MngInfo
-    *mng_info;
-
   MngBox
     default_fb,
     fb,
@@ -5368,36 +5365,10 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
   default_fb.left=0;
   default_fb.right=0;
 
-  /* Open image file.  */
-
-  assert(image_info != (const ImageInfo *) NULL);
-  assert(image_info->signature == MagickCoreSignature);
-  (void) LogMagickEvent(TraceEvent,GetMagickModule(),"%s",
-     image_info->filename);
-  assert(exception != (ExceptionInfo *) NULL);
-  assert(exception->signature == MagickCoreSignature);
-  logging=LogMagickEvent(CoderEvent,GetMagickModule(),"Enter ReadMNGImage()");
-  image=AcquireImage(image_info,exception);
-  mng_info=(MngInfo *) NULL;
-  status=OpenBlob(image_info,image,ReadBinaryBlobMode,exception);
-
-  if (status == MagickFalse)
-    return((Image *) NULL);
-
-  first_mng_object=MagickFalse;
-  skipping_loop=(-1);
-
-  /* Allocate a MngInfo structure.  */
-
-  mng_info=(MngInfo *) AcquireMagickMemory(sizeof(MngInfo));
-
-  if (mng_info == (MngInfo *) NULL)
-    ThrowReaderException(ResourceLimitError,"MemoryAllocationFailed");
-
-  /* Initialize members of the MngInfo structure.  */
+  logging=LogMagickEvent(CoderEvent,GetMagickModule(),
+    "  Enter ReadOneMNGImage()");
 
-  (void) ResetMagickMemory(mng_info,0,sizeof(MngInfo));
-  mng_info->image=image;
+  image=mng_info->image;
 
   if (LocaleCompare(image_info->magick,"MNG") == 0)
     {
@@ -5418,6 +5389,7 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
       mng_info->exists[0]=MagickTrue;
     }
 
+  skipping_loop=(-1);
   first_mng_object=MagickTrue;
   mng_type=0;
 #if defined(MNG_INSERT_LAYERS)
@@ -6039,7 +6011,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
                     if (GetNextImageInList(image) == (Image *) NULL)
                       {
                         image=DestroyImageList(image);
-                        mng_info=MngInfoFreeStruct(mng_info);
                         return((Image *) NULL);
                       }
 
@@ -6616,7 +6587,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
                     if (GetNextImageInList(image) == (Image *) NULL)
                       {
                         image=DestroyImageList(image);
-                        MngInfoFreeStruct(mng_info);
                         return((Image *) NULL);
                       }
 
@@ -6669,7 +6639,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
               if (GetNextImageInList(image) == (Image *) NULL)
                 {
                   image=DestroyImageList(image);
-                  mng_info=MngInfoFreeStruct(mng_info);
                   return((Image *) NULL);
                 }
 
@@ -6718,7 +6687,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
             if (GetNextImageInList(image) == (Image *) NULL)
               {
                 image=DestroyImageList(image);
-                mng_info=MngInfoFreeStruct(mng_info);
                 return((Image *) NULL);
               }
 
@@ -6788,7 +6756,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
           (void) LogMagickEvent(CoderEvent,GetMagickModule(),
             "exit ReadJNGImage() with error");
 
-        mng_info=MngInfoFreeStruct(mng_info);
         return((Image *) NULL);
       }
 
@@ -6796,7 +6763,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
       {
         (void) CloseBlob(image);
         image=DestroyImageList(image);
-        mng_info=MngInfoFreeStruct(mng_info);
         return((Image *) NULL);
       }
 
@@ -6910,7 +6876,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
                 if (GetNextImageInList(image) == (Image *) NULL)
                   {
                     image=DestroyImageList(image);
-                    mng_info=MngInfoFreeStruct(mng_info);
                     return((Image *) NULL);
                   }
 
@@ -6993,7 +6958,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
                     (next == (Quantum *) NULL))
                   {
                      image=DestroyImageList(image);
-                     mng_info=MngInfoFreeStruct(mng_info);
                      ThrowReaderException(ResourceLimitError,
                        "MemoryAllocationFailed");
                   }
@@ -7491,7 +7455,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
           if (GetNextImageInList(image) == (Image *) NULL)
             {
               image=DestroyImageList(image);
-              mng_info=MngInfoFreeStruct(mng_info);
 
               if (logging != MagickFalse)
                 (void) LogMagickEvent(CoderEvent,GetMagickModule(),
@@ -7574,7 +7537,6 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
       if (image != (Image *) NULL)
         image=DestroyImageList(image);
 
-      mng_info=MngInfoFreeStruct(mng_info);
       return((Image *) NULL);
     }
 
@@ -7709,9 +7671,66 @@ static Image *ReadMNGImage(const ImageInfo *image_info,
       }
    }
 
-  image=GetFirstImageInList(image);
+  if (logging != MagickFalse)
+    (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+      "  exit ReadOneMNGImage();");
+
+  return(image);
+}
+
+static Image *ReadMNGImage(const ImageInfo *image_info,
+     ExceptionInfo *exception)
+{
+  Image
+    *image;
+
+  MagickBooleanType
+    logging,
+    status;
+
+  MngInfo
+    *mng_info;
+
+  /* Open image file.  */
+
+  assert(image_info != (const ImageInfo *) NULL);
+  assert(image_info->signature == MagickCoreSignature);
+  (void) LogMagickEvent(TraceEvent,GetMagickModule(),"%s",
+     image_info->filename);
+  assert(exception != (ExceptionInfo *) NULL);
+  assert(exception->signature == MagickCoreSignature);
+  logging=LogMagickEvent(CoderEvent,GetMagickModule(),"Enter ReadMNGImage()");
+  image=AcquireImage(image_info,exception);
+  mng_info=(MngInfo *) NULL;
+  status=OpenBlob(image_info,image,ReadBinaryBlobMode,exception);
+
+  if (status == MagickFalse)
+    return((Image *) NULL);
+
+  /* Allocate a MngInfo structure.  */
+
+  mng_info=(MngInfo *) AcquireMagickMemory(sizeof(MngInfo));
+
+  if (mng_info == (MngInfo *) NULL)
+    ThrowReaderException(ResourceLimitError,"MemoryAllocationFailed");
+
+  /* Initialize members of the MngInfo structure.  */
+
+  (void) ResetMagickMemory(mng_info,0,sizeof(MngInfo));
+  mng_info->image=image;
+  image=ReadOneMNGImage(mng_info,image_info,exception);
   mng_info=MngInfoFreeStruct(mng_info);
 
+  if (image == (Image *) NULL)
+    {
+      if (logging != MagickFalse)
+        (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+          "exit ReadMNGImage() with error");
+
+      return((Image *) NULL);
+    }
+  (void) CloseBlob(image);
+
   if (logging != MagickFalse)
     (void) LogMagickEvent(CoderEvent,GetMagickModule(),"exit ReadMNGImage()");