GraphicsMagick: MSL: Add more input image validations and use ma...

changeset dc28ec43d483 in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=dc28ec43d483
summary: MSL: Add more input image validations and use macros to reduce repeated code.

diffstat:

 ChangeLog                              |   11 +
 VisualMagick/installer/inc/version.isx |    4 +-
 coders/msl.c                           |  603 +++++++-------------------------
 locale/C.mgk                           |   22 +-
 magick/transform.c                     |    4 +-
 magick/version.h                       |    4 +-
 www/Changelog.html                     |   10 +
 7 files changed, 170 insertions(+), 488 deletions(-)

diffs (truncated from 1232 to 500 lines):

diff -r 6a386072d064 -r dc28ec43d483 ChangeLog
--- a/ChangeLog	Sat Jul 17 08:24:57 2021 -0500
+++ b/ChangeLog	Sun Jul 18 09:58:45 2021 -0500
@@ -1,3 +1,14 @@
+2021-07-18  Bob Friesenhahn  <[email protected]>
+
+        * coders/msl.c (MSLStartElement): Use macros to simplify
+        validations and reduce repeated code fragments. Add validations
+        for image size and pixels present where applicable.  Fixes
+        oss-fuzz 36224 "graphicsmagick:coder_MSL_fuzzer: Timeout in
+        coder_MSL_fuzzer".
+
+        * magick/transform.c (RollImage): Assert that image rows and
+        columns are not zero.
+
 2021-07-16  Bob Friesenhahn  <[email protected]>
 
         * coders/jp2.c (initialize_jasper): Update for the latest version
diff -r 6a386072d064 -r dc28ec43d483 VisualMagick/installer/inc/version.isx
--- a/VisualMagick/installer/inc/version.isx	Sat Jul 17 08:24:57 2021 -0500
+++ b/VisualMagick/installer/inc/version.isx	Sun Jul 18 09:58:45 2021 -0500
@@ -10,5 +10,5 @@
 
 #define public MagickPackageName "GraphicsMagick"
 #define public MagickPackageVersion "1.4"
-#define public MagickPackageVersionAddendum ".020210716"
-#define public MagickPackageReleaseDate "snapshot-20210716"
+#define public MagickPackageVersionAddendum ".020210718"
+#define public MagickPackageReleaseDate "snapshot-20210718"
diff -r 6a386072d064 -r dc28ec43d483 coders/msl.c
--- a/coders/msl.c	Sat Jul 17 08:24:57 2021 -0500
+++ b/coders/msl.c	Sun Jul 18 09:58:45 2021 -0500
@@ -580,6 +580,36 @@
     }
 }
 
+#define MSL_BREAK_IF_IMAGE_NULL(_image)                        \
+  if (_image == (Image *) NULL)                                \
+    {                                                          \
+      ThrowException(msl_info->exception,OptionError,          \
+                     NoImagesDefined,(char *) name);           \
+      break;                                                   \
+    }
+
+#define MSL_BREAK_IF_IMAGE_ZERO_SIZE(_image)                            \
+  if ((_image->rows) == 0 ||                                            \
+      (_image->columns == 0))                                           \
+    {                                                                   \
+      ThrowException(msl_info->exception,OptionError,                   \
+                     NonzeroWidthAndHeightRequired,(char *) name);      \
+      break;                                                            \
+    }
+
+#define MSL_BREAK_IF_IMAGE_PIXEL_CACHE_NOT_PRESENT(_image)  \
+  if (!GetPixelCachePresent(_image))                                    \
+    {                                                                   \
+      ThrowException(msl_info->exception,OptionError,                   \
+                     NoImagesDefined,(char *) name);                    \
+      break;                                                            \
+    }
+
+#define MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(_image)   \
+  MSL_BREAK_IF_IMAGE_NULL(_image);                    \
+  MSL_BREAK_IF_IMAGE_ZERO_SIZE(_image);               \
+  MSL_BREAK_IF_IMAGE_PIXEL_CACHE_NOT_PRESENT(_image)
+
 static void
 MSLStartElement(void *context,const xmlChar *name,
                 const xmlChar **attributes)
@@ -628,23 +658,14 @@
             double  radius = 0.0,
               sigma = 1.0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,NoImagesDefined,
-                               (char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes != (const xmlChar **) NULL)
               {
                 for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
                   {
                     keyword=(const char *) attributes[i++];
-                    if (msl_info->attributes[n] == (Image *) NULL)
-                      {
-                        ThrowException(msl_info->exception,OptionError,
-                                       NoImagesDefined,(char *) keyword);
-                        break;
-                      }
+                    MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                     value=TranslateText(msl_info->image_info[n],
                                         msl_info->attributes[n],
                                         (char *) attributes[i]);
@@ -711,23 +732,14 @@
             width = height = 6;  /* this is the value that Magick++ uses */
             x = y = 0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,NoImagesDefined,
-                               (char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes != (const xmlChar **) NULL)
               {
                 for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
                   {
                     keyword=(const char *) attributes[i++];
-                    if (msl_info->attributes[n] == (Image *) NULL)
-                      {
-                        ThrowException(msl_info->exception,OptionError,
-                                       NoImagesDefined,(char *) keyword);
-                        break;
-                      }
+                    MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                     value=TranslateText(msl_info->image_info[n],
                                         msl_info->attributes[n],
                                         (char *) attributes[i]);
@@ -836,23 +848,14 @@
             double  radius = 0.0,
               sigma = 1.0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes != (const xmlChar **) NULL)
               {
                 for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
                   {
                     keyword=(const char *) attributes[i++];
-                    if (msl_info->attributes[n] == (Image *) NULL)
-                      {
-                        ThrowException(msl_info->exception,OptionError,
-                                       NoImagesDefined,(char *) keyword);
-                        break;
-                      }
+                    MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                     value=TranslateText(msl_info->image_info[n],
                                         msl_info->attributes[n],
                                         (char *) attributes[i]);
@@ -920,23 +923,14 @@
             height=msl_info->image[n]->rows;
             x = y = 0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes == (const xmlChar **) NULL)
               break;
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
               {
                 keyword=(const char *) attributes[i++];
-                if (msl_info->attributes[n] == (Image *) NULL)
-                  {
-                    ThrowException(msl_info->exception,OptionError,
-                                   NoImagesDefined,(char *) keyword);
-                    break;
-                  }
+                MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                 value=TranslateText(msl_info->image_info[n],
                                     msl_info->attributes[n],
                                     (char *) attributes[i]);
@@ -1051,23 +1045,14 @@
 
             x = y = 0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes == (const xmlChar **) NULL)
               break;
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
               {
                 keyword=(const char *) attributes[i++];
-                if (msl_info->attributes[n] == (Image *) NULL)
-                  {
-                    ThrowException(msl_info->exception,OptionError,
-                                   NoImagesDefined,(char *) keyword);
-                    break;
-                  }
+                MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                 value=TranslateText(msl_info->image_info[n],
                                     msl_info->attributes[n],
                                     (char *) attributes[i]);
@@ -1259,28 +1244,19 @@
           }
         else if (LocaleCompare((char *) name,"crop") == 0)
           {
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             /* init the values */
             width=msl_info->image[n]->columns;
             height=msl_info->image[n]->rows;
             x = y = 0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
             if (attributes == (const xmlChar **) NULL)
               break;
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
               {
                 keyword=(const char *) attributes[i++];
-                if (msl_info->attributes[n] == (Image *) NULL)
-                  {
-                    ThrowException(msl_info->exception,OptionError,
-                                   NoImagesDefined,(char *) keyword);
-                    break;
-                  }
+                MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                 value=TranslateText(msl_info->image_info[n],
                                     msl_info->attributes[n],
                                     (char *) attributes[i]);
@@ -1393,12 +1369,7 @@
       {
         if (LocaleCompare((char *) name, "despeckle") == 0)
           {
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
 
             /* no attributes here */
 
@@ -1427,23 +1398,14 @@
           {
             double  radius = 0.0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes != (const xmlChar **) NULL)
               {
                 for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
                   {
                     keyword=(const char *) attributes[i++];
-                    if (msl_info->attributes[n] == (Image *) NULL)
-                      {
-                        ThrowException(msl_info->exception,OptionError,
-                                       NoImagesDefined,(char *) keyword);
-                        break;
-                      }
+                    MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                     value=TranslateText(msl_info->image_info[n],
                                         msl_info->attributes[n],
                                         (char *) attributes[i]);
@@ -1497,23 +1459,14 @@
             double  radius = 0.0,
               sigma = 1.0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes != (const xmlChar **) NULL)
               {
                 for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
                   {
                     keyword=(const char *) attributes[i++];
-                    if (msl_info->attributes[n] == (Image *) NULL)
-                      {
-                        ThrowException(msl_info->exception,OptionError,
-                                       NoImagesDefined,(char *) keyword);
-                        break;
-                      }
+                    MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                     value=TranslateText(msl_info->image_info[n],
                                         msl_info->attributes[n],
                                         (char *) attributes[i]);
@@ -1576,12 +1529,7 @@
           }
         else if (LocaleCompare((char *) name, "enhance") == 0)
           {
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
 
             /* no attributes here */
 
@@ -1627,12 +1575,7 @@
       {
         if (LocaleCompare((char *) name, "flatten") == 0)
           {
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
 
             /* no attributes here */
 
@@ -1652,12 +1595,7 @@
           }
         else if (LocaleCompare((char *) name, "flip") == 0)
           {
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
 
             /* no attributes here */
 
@@ -1677,12 +1615,7 @@
           }
         else if (LocaleCompare((char *) name, "flop") == 0)
           {
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
 
             /* no attributes here */
 
@@ -1702,27 +1635,18 @@
           }
         else if (LocaleCompare((char *) name,"frame") == 0)
           {
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             /* init the values */
             width = height = 25;  /* these are the values that Magick++ uses */
             x = y = 6;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
             if (attributes != (const xmlChar **) NULL)
               {
                 for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
                   {
                     keyword=(const char *) attributes[i++];
-                    if (msl_info->attributes[n] == (Image *) NULL)
-                      {
-                        ThrowException(msl_info->exception,OptionError,
-                                       NoImagesDefined,(char *) keyword);
-                        break;
-                      }
+                    MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                     value=TranslateText(msl_info->image_info[n],
                                         msl_info->attributes[n],
                                         (char *) attributes[i]);
@@ -1883,12 +1807,8 @@
             double
               gammaRed = 0, gammaGreen = 0, gammaBlue = 0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes == (const xmlChar **) NULL)
               break;
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
@@ -1956,12 +1876,9 @@
           }
         else if (LocaleCompare((char *) name,"get") == 0)
           {
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+
+            MSL_BREAK_IF_IMAGE_NULL(msl_info->image[n]);
+
             if (attributes == (const xmlChar **) NULL)
               break;
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
@@ -2054,12 +1971,7 @@
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
               {
                 keyword=(const char *) attributes[i++];
-                if (msl_info->attributes[n] == (Image *) NULL)
-                  {
-                    ThrowException(msl_info->exception,OptionError,
-                                   NoImagesDefined,(char *) keyword);
-                    break;
-                  }
+                MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                 value=TranslateText(msl_info->image_info[n],
                                     msl_info->attributes[n],
                                     (char *) attributes[i]);
@@ -2171,23 +2083,14 @@
             /* init the values */
             double  amount = 0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes == (const xmlChar **) NULL)
               break;
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
               {
                 keyword=(const char *) attributes[i++];
-                if (msl_info->attributes[n] == (Image *) NULL)
-                  {
-                    ThrowException(msl_info->exception,OptionError,
-                                   NoImagesDefined,(char *) keyword);
-                    break;
-                  }
+                MSL_BREAK_IF_IMAGE_NULL(msl_info->attributes[n]);
                 value=TranslateText(msl_info->image_info[n],
                                     msl_info->attributes[n],
                                     (char *) attributes[i]);
@@ -2244,12 +2147,7 @@
       {
         if (LocaleCompare((char *) name, "magnify") == 0)
           {
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
 
             /* no attributes here */
 
@@ -2272,23 +2170,14 @@
             /* init the values */
             unsigned int  radius = 0;
 
-            if (msl_info->image[n] == (Image *) NULL)
-              {
-                ThrowException(msl_info->exception,OptionError,
-                               NoImagesDefined,(char *) name);
-                break;
-              }
+            MSL_BREAK_IF_IMAGE_NOT_INSTANTIATED(msl_info->image[n]);
+
             if (attributes == (const xmlChar **) NULL)
               break;
             for (i=0; (attributes[i] != (const xmlChar *) NULL); i++)
               {
                 keyword=(const char *) attributes[i++];
-                if (msl_info->attributes[n] == (Image *) NULL)
-                  {
-                    ThrowException(msl_info->exception,OptionError,
-                                   NoImagesDefined,(char *) keyword);
-                    break;
-                  }