GraphicsMagick: ReadJBIGImage(): Set maximum image dimensions ba...

GraphicsMagick Commits <[email protected]> Sun, 10 Dec 2023 09:08:23 -0600
Newsgroups gmane.comp.video.graphicsmagick.cvs
Message-ID <mailman.11118.1702220917.7940.graphicsmagick-commit@lists.sourceforge.net>
changeset a91d0ca1311e in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=a91d0ca1311e
summary: ReadJBIGImage(): Set maximum image dimensions based on resource limits.

diffstat:

 ChangeLog                              |   7 ++++
 VisualMagick/installer/inc/version.isx |   4 +-
 coders/jbig.c                          |  59 ++++++++++++++++++++++++++++++---
 magick/resource.c                      |  34 +++++++++---------
 magick/resource.h                      |  10 +++++
 magick/version.h                       |   4 +-
 www/Changelog.html                     |   9 +++++
 7 files changed, 100 insertions(+), 27 deletions(-)

diffs (275 lines):

diff -r 71e92633585d -r a91d0ca1311e ChangeLog
--- a/ChangeLog	Sat Dec 09 18:35:18 2023 -0600
+++ b/ChangeLog	Sun Dec 10 09:08:20 2023 -0600
@@ -1,3 +1,10 @@
+2023-12-10  Bob Friesenhahn  <[email protected]>
+
+	* coders/jbig.c (ReadJBIGImage): Attempt to set maximum image
+	dimensions based on resource limits before calling jbg_dec_in() so
+	it will quit on excessively large images.  This often works, but
+	not all the time.
+
 2023-12-09  Bob Friesenhahn  <[email protected]>
 
 	* fuzzing/oss-fuzz-build.sh: Add -DJPEGXL_ENABLE_SKCMS=false to
diff -r 71e92633585d -r a91d0ca1311e VisualMagick/installer/inc/version.isx
--- a/VisualMagick/installer/inc/version.isx	Sat Dec 09 18:35:18 2023 -0600
+++ b/VisualMagick/installer/inc/version.isx	Sun Dec 10 09:08:20 2023 -0600
@@ -10,5 +10,5 @@
 
 #define public MagickPackageName "GraphicsMagick"
 #define public MagickPackageVersion "1.4"
-#define public MagickPackageVersionAddendum ".020231209"
-#define public MagickPackageReleaseDate "snapshot-20231209"
+#define public MagickPackageVersionAddendum ".020231210"
+#define public MagickPackageReleaseDate "snapshot-20231210"
diff -r 71e92633585d -r a91d0ca1311e coders/jbig.c
--- a/coders/jbig.c	Sat Dec 09 18:35:18 2023 -0600
+++ b/coders/jbig.c	Sun Dec 10 09:08:20 2023 -0600
@@ -43,6 +43,7 @@
 #include "magick/magick.h"
 #include "magick/monitor.h"
 #include "magick/pixel_cache.h"
+#include "magick/resource.h"
 #include "magick/utility.h"
 
 /*
@@ -134,10 +135,48 @@
     Initialize JBIG toolkit.
   */
   jbg_dec_init(&jbig_info);
-  jbg_dec_maxsize(&jbig_info,(unsigned long) image->columns,
-                  (unsigned long) image->rows);
-  image->columns= jbg_dec_getwidth(&jbig_info);
-  image->rows= jbg_dec_getheight(&jbig_info);
+
+  /*
+    Attempt to set maximum image dimensions based on resource limits.
+
+    This does work in normal cases, but in other cases the first call
+    to jbg_dec_in() takes a very long time, and it returns large image
+    dimensions anyway.
+  */
+  {
+    magick_int64_t
+      width_limit,
+      height_limit,
+      pixels_limit;
+
+    width_limit = GetMagickResourceLimit(WidthResource);
+    height_limit = GetMagickResourceLimit(HeightResource);
+    pixels_limit = GetMagickResourceLimit(PixelsResource);
+
+    if (MagickResourceInfinity != width_limit)
+      if ((image->columns == 0) || (image->columns > (unsigned long) width_limit))
+        image->columns = (unsigned long) width_limit;
+
+    if (MagickResourceInfinity != height_limit)
+      if ((image->rows == 0) || (image->rows > (unsigned long) height_limit))
+        image->rows = (unsigned long) height_limit;
+
+    if ((MagickResourceInfinity != pixels_limit) &&
+        ((magick_int64_t) (image->columns*image->rows)) > pixels_limit)
+      {
+        magick_int64_t max_dimension = sqrt((double) pixels_limit);
+        image->columns = (unsigned long) max_dimension;
+        image->rows = (unsigned long) max_dimension;
+      }
+
+    if (image->logging)
+      (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                            "JBIG: Setting maximum dimensions %lux%lu",
+                            image->columns, image->rows);
+    jbg_dec_maxsize(&jbig_info,(unsigned long) image->columns,
+                    (unsigned long) image->rows);
+  }
+
   image->depth=1;
   /*
     Read JBIG file.
@@ -147,6 +186,9 @@
     ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,image);
   status=JBG_EAGAIN;
   /* FIXME: Should handle JBG_EOK_INTR for multi-resolution support */
+  if (image->logging)
+    (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                          "JBIG: Entering jbg_dec_in() decode loop...");
   do
     {
       length=(long) ReadBlob(image,MaxBufferSize,(char *) buffer);
@@ -159,7 +201,7 @@
           status=jbg_dec_in(&jbig_info,p,length,&count);
           if (image->logging)
             (void) LogMagickEvent(CoderEvent,GetMagickModule(),
-                                  "jbg_dec_in() returns 0x%02x (\"%s\")",
+                                  "JBIG: jbg_dec_in() returns 0x%02x (\"%s\")",
                                   status, jbg_strerror(status));
           p+=count;
           length-=count;
@@ -196,6 +238,11 @@
   image->is_grayscale=MagickTrue;
   image->is_monochrome=MagickTrue;
   image->colorspace=GRAYColorspace;
+  if (image->logging)
+    (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                          "JBIG: %lux%lu, resolution %gx%g",
+                          image->columns, image->rows,
+                          image->x_resolution,image->y_resolution);
   if (image_info->ping)
     {
       jbg_dec_free(&jbig_info);
@@ -281,7 +328,7 @@
     description[]="Joint Bi-level Image experts Group interchange format";
 
   static const char
-    version[]="JBIG-Kit " JBG_VERSION;
+    version[]="JBIG-Kit " JBG_VERSION " (" JBG_LICENCE " license)";
 
   MagickInfo
     *entry;
diff -r 71e92633585d -r a91d0ca1311e magick/resource.c
--- a/magick/resource.c	Sat Dec 09 18:35:18 2023 -0600
+++ b/magick/resource.c	Sun Dec 10 09:08:20 2023 -0600
@@ -47,7 +47,7 @@
 /*
   Define  declarations.
 */
-#define ResourceInfinity ((magick_int64_t) (~((magick_uint64_t) 0) >> 1))
+/* #define MagickResourceInfinity ((magick_int64_t) (~((magick_uint64_t) 0) >> 1)) */
 #define ResourceInfoMaxIndex ((unsigned int) (sizeof(resource_info)/sizeof(resource_info[0])-1))
 
 /*
@@ -96,17 +96,17 @@
 static ResourceInfo
   resource_info[] =
   {
-    { "",       "",  "",                    0, 0,     ResourceInfinity, AbsoluteLimit, 0  },
-    { "disk",   "B", "MAGICK_LIMIT_DISK",   0, 0,     ResourceInfinity, SummationLimit, 0 },
-    { "files",  "",  "MAGICK_LIMIT_FILES",  0, 32,    256,              SummationLimit, 0 },
-    { "map",    "B", "MAGICK_LIMIT_MAP",    0, 0,     ResourceInfinity, SummationLimit, 0 },
-    { "memory", "B", "MAGICK_LIMIT_MEMORY", 0, 0,     ResourceInfinity, SummationLimit, 0 },
-    { "pixels", "P", "MAGICK_LIMIT_PIXELS", 0, 1,     ResourceInfinity, AbsoluteLimit, 0  },
-    { "threads", "", "OMP_NUM_THREADS",     1, 1,     ResourceInfinity, AbsoluteLimit, 0  },
-    { "width",  "P", "MAGICK_LIMIT_WIDTH",  0, 1,     PIXEL_LIMIT,      AbsoluteLimit, 0  },
-    { "height", "P", "MAGICK_LIMIT_HEIGHT", 0, 1,     PIXEL_LIMIT,      AbsoluteLimit, 0  },
-    { "read",   "B", "MAGICK_LIMIT_READ",   0, 4096,  ResourceInfinity, AbsoluteLimit, 0  },
-    { "write",  "B", "MAGICK_LIMIT_WRITE",  0, 4096,  ResourceInfinity, AbsoluteLimit, 0  }
+    { "",       "",  "",                    0, 0,     MagickResourceInfinity, AbsoluteLimit, 0  },
+    { "disk",   "B", "MAGICK_LIMIT_DISK",   0, 0,     MagickResourceInfinity, SummationLimit, 0 },
+    { "files",  "",  "MAGICK_LIMIT_FILES",  0, 32,    256,                    SummationLimit, 0 },
+    { "map",    "B", "MAGICK_LIMIT_MAP",    0, 0,     MagickResourceInfinity, SummationLimit, 0 },
+    { "memory", "B", "MAGICK_LIMIT_MEMORY", 0, 0,     MagickResourceInfinity, SummationLimit, 0 },
+    { "pixels", "P", "MAGICK_LIMIT_PIXELS", 0, 1,     MagickResourceInfinity, AbsoluteLimit, 0  },
+    { "threads", "", "OMP_NUM_THREADS",     1, 1,     MagickResourceInfinity, AbsoluteLimit, 0  },
+    { "width",  "P", "MAGICK_LIMIT_WIDTH",  0, 1,     PIXEL_LIMIT,            AbsoluteLimit, 0  },
+    { "height", "P", "MAGICK_LIMIT_HEIGHT", 0, 1,     PIXEL_LIMIT,            AbsoluteLimit, 0  },
+    { "read",   "B", "MAGICK_LIMIT_READ",   0, 4096,  MagickResourceInfinity, AbsoluteLimit, 0  },
+    { "write",  "B", "MAGICK_LIMIT_WRITE",  0, 4096,  MagickResourceInfinity, AbsoluteLimit, 0  }
   };
 
 /*
@@ -177,7 +177,7 @@
               Limit depends only on the currently requested size.
             */
             value=info->value;
-            if ((info->maximum != ResourceInfinity) &&
+            if ((info->maximum != MagickResourceInfinity) &&
                 (size > (magick_uint64_t) info->maximum))
               status=MagickFail;
             break;
@@ -190,7 +190,7 @@
             */
             LockSemaphoreInfo(info->semaphore);
             value=info->value+size;
-            if ((info->maximum != ResourceInfinity) &&
+            if ((info->maximum != MagickResourceInfinity) &&
                 (value > (magick_uint64_t) info->maximum))
               {
                 value=info->value;
@@ -211,7 +211,7 @@
             f_size[MaxTextExtent],
             f_value[MaxTextExtent];
 
-          if (info->maximum == ResourceInfinity)
+          if (info->maximum == MagickResourceInfinity)
             {
               strlcpy(f_limit,"Unlimited",sizeof(f_limit));
             }
@@ -799,7 +799,7 @@
             f_size[MaxTextExtent],
             f_value[MaxTextExtent];
 
-          if (info->maximum == ResourceInfinity)
+          if (info->maximum == MagickResourceInfinity)
             {
               strlcpy(f_limit,"Unlimited",sizeof(f_limit));
             }
@@ -881,7 +881,7 @@
         limit[MaxTextExtent];
 
       LockSemaphoreInfo(resource_info[index].semaphore);
-      if (resource_info[index].maximum == ResourceInfinity)
+      if (resource_info[index].maximum == MagickResourceInfinity)
         {
           strlcpy(limit,"Unlimited",sizeof(limit));
         }
diff -r 71e92633585d -r a91d0ca1311e magick/resource.h
--- a/magick/resource.h	Sat Dec 09 18:35:18 2023 -0600
+++ b/magick/resource.h	Sun Dec 10 09:08:20 2023 -0600
@@ -33,6 +33,7 @@
   WriteResource        /* Maximum amount of uncompressed file data which may be written to one file */
 } ResourceType;
 
+
 /*
   Method declarations.
 */
@@ -50,6 +51,15 @@
   InitializeMagickResources(void),
   LiberateMagickResource(const ResourceType type,const magick_uint64_t size);
 
+#if defined(MAGICK_IMPLEMENTATION)
+
+/*
+  Define  declarations.
+*/
+#define MagickResourceInfinity ((magick_int64_t) (~((magick_uint64_t) 0) >> 1))
+
+#endif /* MAGICK_IMPLEMENTATION */
+
 
 #if defined(__cplusplus) || defined(c_plusplus)
 }
diff -r 71e92633585d -r a91d0ca1311e magick/version.h
--- a/magick/version.h	Sat Dec 09 18:35:18 2023 -0600
+++ b/magick/version.h	Sun Dec 10 09:08:20 2023 -0600
@@ -38,8 +38,8 @@
 #define MagickLibVersion  0x272404
 #define MagickLibVersionText  "1.4"
 #define MagickLibVersionNumber 27,24,4
-#define MagickChangeDate   "20231209"
-#define MagickReleaseDate  "snapshot-20231209"
+#define MagickChangeDate   "20231210"
+#define MagickReleaseDate  "snapshot-20231210"
 
 /*
   The MagickLibInterfaceNewest and MagickLibInterfaceOldest defines
diff -r 71e92633585d -r a91d0ca1311e www/Changelog.html
--- a/www/Changelog.html	Sat Dec 09 18:35:18 2023 -0600
+++ b/www/Changelog.html	Sun Dec 10 09:08:20 2023 -0600
@@ -37,6 +37,15 @@
 </div>
 
 <div class="document">
+<p>2023-12-10  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
+<blockquote>
+<ul class="simple">
+<li><p>coders/jbig.c (ReadJBIGImage): Attempt to set maximum image
+dimensions based on resource limits before calling jbg_dec_in() so
+it will quit on excessively large images.  This often works, but
+not all the time.</p></li>
+</ul>
+</blockquote>
 <p>2023-12-09  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
 <blockquote>
 <ul class="simple">