CVE-2019-13311: Missing Release of Memory after Effective Lifetime (CWE-401)
value: ImageMagick 7.0.8-50 Q16 has memory leaks at AcquireMagickMemory because of a wand/mogrify.c error.
CWE description: The product does not sufficiently track and release allocated memory after it has been used, which slowly consumes remaining memory.
CVSS v3 base score: 6.5
Published: 2019-07-05T01:15Z
Repository: https://github.com/ImageMagick/ImageMagick
Language: C
Files changed: MagickWand/mogrify.c, MagickWand/operation.c
Commit date: 2019-07-01 19:53:59 -0400
Commit message: https://github.com/ImageMagick/ImageMagick/issues/1623

Diff: diff --git a/MagickWand/mogrify.c b/MagickWand/mogrify.c
index 150b6792304..9f7eb6b551d 100644
--- a/MagickWand/mogrify.c
+++ b/MagickWand/mogrify.c
@@ -7822,10 +7822,13 @@ WandExport MagickBooleanType MogrifyImageList(ImageInfo *image_info,
             (void) SyncImagesSettings(mogrify_info,*images,exception);
             image=RemoveFirstImageFromList(images);
             clut_image=RemoveFirstImageFromList(images);
             if (clut_image == (Image *) NULL)
               {
+                (void) ThrowMagickException(exception,GetMagickModule(),
+                  OptionError,"ImageSequenceRequired","`%s'",option);
+                image=DestroyImage(image);
                 status=MagickFalse;
                 break;
               }
             (void) ClutImage(image,clut_image,interpolate_method,exception);
             clut_image=DestroyImage(clut_image);
@@ -7894,10 +7897,13 @@ WandExport MagickBooleanType MogrifyImageList(ImageInfo *image_info,
             (void) SyncImagesSettings(mogrify_info,*images,exception);
             image=RemoveFirstImageFromList(images);
             reconstruct_image=RemoveFirstImageFromList(images);
             if (reconstruct_image == (Image *) NULL)
               {
+                (void) ThrowMagickException(exception,GetMagickModule(),
+                  OptionError,"ImageSequenceRequired","`%s'",option);
+                image=DestroyImage(image);
                 status=MagickFalse;
                 break;
               }
             metric=UndefinedErrorMetric;
             option=GetImageOption(mogrify_info,"metric");
@@ -7974,11 +7980,17 @@ WandExport MagickBooleanType MogrifyImageList(ImageInfo *image_info,
               clip_to_self=IsStringFalse(value);  /* deprecated */
 
             new_images=RemoveFirstImageFromList(images);
             source_image=RemoveFirstImageFromList(images);
             if (source_image == (Image *) NULL)
-              break; /* FUTURE - produce Exception, rather than silent fail */
+              {
+                (void) ThrowMagickException(exception,GetMagickModule(),
+                  OptionError,"ImageSequenceRequired","`%s'",option);
+                new_images=DestroyImage(new_images);
+                status=MagickFalse;
+                break;
+              }
 
             /* FUTURE: this should not be here! - should be part of -geometry */
             if (source_image->geometry != (char *) NULL)
               {
                 RectangleInfo
@@ -8226,10 +8238,13 @@ WandExport MagickBooleanType MogrifyImageList(ImageInfo *image_info,
             (void) SyncImagesSettings(mogrify_info,*images,exception);
             image=RemoveFirstImageFromList(images);
             hald_image=RemoveFirstImageFromList(images);
             if (hald_image == (Image *) NULL)
               {
+                (void) ThrowMagickException(exception,GetMagickModule(),
+                  OptionError,"ImageSequenceRequired","`%s'",option);
+                image=DestroyImage(image);
                 status=MagickFalse;
                 break;
               }
             (void) HaldClutImage(image,hald_image,exception);
             hald_image=DestroyImage(hald_image);
@@ -8255,15 +8270,20 @@ WandExport MagickBooleanType MogrifyImageList(ImageInfo *image_info,
             (void) SyncImagesSettings(mogrify_info,*images,exception);
             magnitude_image=RemoveFirstImageFromList(images);
             phase_image=RemoveFirstImageFromList(images);
             if (phase_image == (Image *) NULL)
               {
+                (void) ThrowMagickException(exception,GetMagickModule(),
+                  OptionError,"ImageSequenceRequired","`%s'",option);
+                magnitude_image=DestroyImage(magnitude_image);
                 status=MagickFalse;
                 break;
               }
             fourier_image=InverseFourierTransformImage(magnitude_image,
               phase_image,*option == '-' ? MagickTrue : MagickFalse,exception);
+            magnitude_image=DestroyImage(magnitude_image);
+            phase_image=DestroyImage(phase_image);
             if (fourier_image == (Image *) NULL)
               break;
             if (*images != (Image *) NULL)
               *images=DestroyImageList(*images);
             *images=fourier_image;
diff --git a/MagickWand/operation.c b/MagickWand/operation.c
index b8745743469..57a29ac91f6 100644
--- a/MagickWand/operation.c
+++ b/MagickWand/operation.c
@@ -3819,14 +3819,20 @@ WandPrivate MagickBooleanType CLIListOperatorImages(MagickCLI *cli_wand,
           /* FUTURE - make this a compose option, and thus can be used
              with layers compose or even compose last image over all other
              _images.
           */
           new_images=RemoveFirstImageFromList(&_images);
-          clut_image=RemoveLastImageFromList(&_images);
+          clut_image=RemoveFirstImageFromList(&_images);
           /* FUTURE - produce Exception, rather than silent fail */
           if (clut_image == (Image *) NULL)
-            break;
+            {
+              (void) ThrowMagickException(_exception,GetMagickModule(),
+                OptionError,"ImageSequenceRequired","`%s'",option);
+              new_images=DestroyImage(new_images);
+              status=MagickFalse;
+              break;
+            }
           (void) ClutImage(new_images,clut_image,new_images->interpolate,
             _exception);
           clut_image=DestroyImage(clut_image);
           break;
         }
@@ -3866,12 +3872,15 @@ WandPrivate MagickBooleanType CLIListOperatorImages(MagickCLI *cli_wand,
           */
           image=RemoveFirstImageFromList(&_images);
           reconstruct_image=RemoveFirstImageFromList(&_images);
           /* FUTURE - produce Exception, rather than silent fail */
           if (reconstruct_image == (Image *) NULL)
-            { 
+            {
+              (void) ThrowMagickException(_exception,GetMagickModule(),
+                OptionError,"ImageSequenceRequired","`%s'",option);
               image=DestroyImage(image);
+              status=MagickFalse;
               break;
             }
           metric=UndefinedErrorMetric;
           option=GetImageOption(_image_info,"metric");
           if (option != (const char *) NULL)
@@ -3929,11 +3938,17 @@ WandPrivate MagickBooleanType CLIListOperatorImages(MagickCLI *cli_wand,
             clip_to_self=IsStringFalse(value);  /* deprecated */
 
           new_images=RemoveFirstImageFromList(&_images);
           source_image=RemoveFirstImageFromList(&_images);
           if (source_image == (Image *) NULL)
-            break; /* FUTURE - produce Exception, rather than silent fail */
+            {
+              (void) ThrowMagickException(_exception,GetMagickModule(),
+                OptionError,"ImageSequenceRequired","`%s'",option);
+              new_images=DestroyImage(new_images);
+              status=MagickFalse;
+              break;
+            }
 
           /* FUTURE - this should not be here! - should be part of -geometry */
           if (source_image->geometry != (char *) NULL)
             {
               RectangleInfo
@@ -4119,11 +4134,17 @@ WandPrivate MagickBooleanType CLIListOperatorImages(MagickCLI *cli_wand,
             *hald_image;
 
           new_images=RemoveFirstImageFromList(&_images);
           hald_image=RemoveLastImageFromList(&_images);
           if (hald_image == (Image *) NULL)
-            break;
+            {
+              (void) ThrowMagickException(_exception,GetMagickModule(),
+                OptionError,"ImageSequenceRequired","`%s'",option);
+              new_images=DestroyImage(new_images);
+              status=MagickFalse;
+              break;
+            }
           (void) HaldClutImage(new_images,hald_image,_exception);
           hald_image=DestroyImage(hald_image);
           break;
         }
       CLIWandExceptionBreak(OptionError,"UnrecognizedOption",option);
@@ -4134,19 +4155,24 @@ WandPrivate MagickBooleanType CLIListOperatorImages(MagickCLI *cli_wand,
         {
           Image
             *magnitude_image,
             *phase_image;
 
-           magnitude_image=RemoveFirstImageFromList(&_images);
-           phase_image=RemoveFirstImageFromList(&_images);
-          /* FUTURE - produce Exception, rather than silent fail */
-           if (phase_image == (Image *) NULL)
-             break;
-           new_images=InverseFourierTransformImage(magnitude_image,phase_image,
-             IsNormalOp,_exception);
-           magnitude_image=DestroyImage(magnitude_image);
-           phase_image=DestroyImage(phase_image);
+          magnitude_image=RemoveFirstImageFromList(&_images);
+          phase_image=RemoveFirstImageFromList(&_images);
+          if (phase_image == (Image *) NULL)
+            {
+              (void) ThrowMagickException(_exception,GetMagickModule(),
+                OptionError,"ImageSequenceRequired","`%s'",option);
+              magnitude_image=DestroyImage(magnitude_image);
+              status=MagickFalse;
+              break;
+            }
+          new_images=InverseFourierTransformImage(magnitude_image,phase_image,
+            IsNormalOp,_exception);
+          magnitude_image=DestroyImage(magnitude_image);
+          phase_image=DestroyImage(phase_image);
           break;
         }
       if (LocaleCompare("insert",option+1) == 0)
         {
           Image

Vulnerable code:               break; /* FUTURE - produce Exception, rather than silent fail */
          clut_image=RemoveLastImageFromList(&_images);
            break;
            { 
            break; /* FUTURE - produce Exception, rather than silent fail */
            break;
           magnitude_image=RemoveFirstImageFromList(&_images);
           phase_image=RemoveFirstImageFromList(&_images);
          /* FUTURE - produce Exception, rather than silent fail */
           if (phase_image == (Image *) NULL)
             break;
           new_images=InverseFourierTransformImage(magnitude_image,phase_image,
             IsNormalOp,_exception);
           magnitude_image=DestroyImage(magnitude_image);
           phase_image=DestroyImage(phase_image);
Fixed code:                 (void) ThrowMagickException(exception,GetMagickModule(),
                  OptionError,"ImageSequenceRequired","`%s'",option);
                image=DestroyImage(image);
                (void) ThrowMagickException(exception,GetMagickModule(),
                  OptionError,"ImageSequenceRequired","`%s'",option);
                image=DestroyImage(image);
              {
                (void) ThrowMagickException(exception,GetMagickModule(),
                  OptionError,"ImageSequenceRequired","`%s'",option);
                new_images=DestroyImage(new_images);
                status=MagickFalse;
                break;
              }
                (void) ThrowMagickException(exception,GetMagickModule(),
                  OptionError,"ImageSequenceRequired","`%s'",option);
                image=DestroyImage(image);
                (void) ThrowMagickException(exception,GetMagickModule(),
                  OptionError,"ImageSequenceRequired","`%s'",option);
                magnitude_image=DestroyImage(magnitude_image);
            magnitude_image=DestroyImage(magnitude_image);
            phase_image=DestroyImage(phase_image);
          clut_image=RemoveFirstImageFromList(&_images);
            {
              (void) ThrowMagickException(_exception,GetMagickModule(),
                OptionError,"ImageSequenceRequired","`%s'",option);
              new_images=DestroyImage(new_images);
              status=MagickFalse;
              break;
            }
            {
              (void) ThrowMagickException(_exception,GetMagickModule(),
                OptionError,"ImageSequenceRequired","`%s'",option);
              status=MagickFalse;
            {
              (void) ThrowMagickException(_exception,GetMagickModule(),
                OptionError,"ImageSequenceRequired","`%s'",option);
              new_images=DestroyImage(new_images);
              status=MagickFalse;
              break;
            }
            {
              (void) ThrowMagickException(_exception,GetMagickModule(),
                OptionError,"ImageSequenceRequired","`%s'",option);
              new_images=DestroyImage(new_images);
              status=MagickFalse;
              break;
            }
          magnitude_image=RemoveFirstImageFromList(&_images);
          phase_image=RemoveFirstImageFromList(&_images);
          if (phase_image == (Image *) NULL)
            {
              (void) ThrowMagickException(_exception,GetMagickModule(),
                OptionError,"ImageSequenceRequired","`%s'",option);
              magnitude_image=DestroyImage(magnitude_image);
              status=MagickFalse;
              break;
            }
          new_images=InverseFourierTransformImage(magnitude_image,phase_image,
            IsNormalOp,_exception);
          magnitude_image=DestroyImage(magnitude_image);
          phase_image=DestroyImage(phase_image);