GraphicsMagick: GetGeometry(): Validate that parsed values to no...

changeset 90a7c0b86df3 in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=90a7c0b86df3
summary: GetGeometry(): Validate that parsed values to not under/overflow when cast to integral types.

diffstat:

 ChangeLog          |   4 +++
 magick/utility.c   |  70 ++++++++++++++++++++++++++++++++++++++++-------------
 www/Changelog.html |   3 ++
 3 files changed, 60 insertions(+), 17 deletions(-)

diffs (159 lines):

diff -r 27a561878992 -r 90a7c0b86df3 ChangeLog
--- a/ChangeLog	Sat Apr 08 10:14:29 2023 -0500
+++ b/ChangeLog	Sat Apr 08 12:14:35 2023 -0500
@@ -1,5 +1,9 @@
 2023-04-08  Bob Friesenhahn  <[email protected]>
 
+	* magick/utility.c (GetGeometry): Improve geometry parser to
+	validate that parsed double values do not underflow or overflow
+	when cast to 'unsigned long' or 'long' types.
+
 	* coders/mpc.c (ReadMPCImage): If an attribute appears multiple
 	times in the MPC header, only set it once.
 
diff -r 27a561878992 -r 90a7c0b86df3 magick/utility.c
--- a/magick/utility.c	Sat Apr 08 10:14:29 2023 -0500
+++ b/magick/utility.c	Sat Apr 08 12:14:35 2023 -0500
@@ -1605,6 +1605,8 @@
 %
 %
 */
+#define MagickULongRangeOk(double_val) ((double_val <= (double) ULONG_MAX) && (double_val >= 0.0))
+#define MagickLongRangeOk(double_val) ((double_val <= (double) LONG_MAX) && (double_val >= (double) LONG_MIN))
 MagickExport int GetGeometry(const char *image_geometry,long *x,long *y,
   unsigned long *width,unsigned long *height)
 {
@@ -1743,8 +1745,12 @@
       count=MagickStrToD(p,&q,&double_val);
       if (count)
         {
-          bounds.width=(unsigned long) floor(double_val+0.5);
-          flags|=WidthValue;
+          double_val=floor(double_val+0.5);
+          if (MagickULongRangeOk(double_val))
+            {
+              bounds.width=(unsigned long) double_val;
+              flags|=WidthValue;
+            }
         }
       if ((*q == 'x') || (*q == 'X') || ((flags & AreaValue) && (*q == '\0')))
         p=q;
@@ -1753,9 +1759,13 @@
           count=MagickStrToD(p,&p,&double_val);
           if (count)
             {
-              bounds.width=(unsigned long) floor(double_val+0.5);
-              bounds.height=bounds.width;
-              flags|=HeightValue;
+              double_val=floor(double_val+0.5);
+              if (MagickULongRangeOk(double_val))
+                {
+                  bounds.width=(unsigned long) double_val;
+                  bounds.height=bounds.width;
+                  flags|=HeightValue;
+                }
             }
         }
     }
@@ -1769,8 +1779,12 @@
       count=MagickStrToD(p,&p,&double_val);
       if (count)
         {
-          bounds.height=(unsigned long) floor(double_val+0.5);
-          flags|=HeightValue;
+          double_val=floor(double_val+0.5);
+          if (MagickULongRangeOk(double_val))
+            {
+              bounds.height=(unsigned long) double_val;
+              flags|=HeightValue;
+            }
         }
     }
   if ((*p == '+') || (*p == '-'))
@@ -1783,7 +1797,15 @@
           p++;
           q=p;
           count=MagickStrToD(p,&p,&double_val);
-          bounds.x=(long) ceil(double_val-0.5);
+          if (count)
+            {
+              double_val=ceil(double_val-0.5);
+              if (MagickLongRangeOk(double_val))
+                {
+                  bounds.x=(long) double_val;
+                  flags|=XValue;
+                }
+            }
         }
       else
         {
@@ -1792,12 +1814,15 @@
           count=MagickStrToD(p,&p,&double_val);
           if (count)
             {
-              bounds.x=(long) ceil(-double_val-0.5);
-              flags|=XNegative;
+              double_val=ceil(-double_val-0.5);
+              if (MagickLongRangeOk(double_val))
+                {
+                  bounds.x=(long) double_val;
+                  flags|=XValue;
+                  flags|=XNegative;
+                }
             }
         }
-      if (count)
-        flags|=XValue;
       if ((*p == '+') || (*p == '-'))
         {
           /*
@@ -1808,7 +1833,15 @@
               p++;
               q=p;
               count=MagickStrToD(p,&p,&double_val);
-              bounds.y=(long) ceil(double_val-0.5);
+              if (count)
+                {
+                  double_val = ceil(double_val-0.5);
+                  if (MagickLongRangeOk(double_val))
+                    {
+                      bounds.y=(long) double_val;
+                      flags|=YValue;
+                    }
+                }
             }
           else
             {
@@ -1817,12 +1850,15 @@
               count=MagickStrToD(p,&p,&double_val);
               if (count)
                 {
-                  bounds.y=(long) ceil(-double_val-0.5);
-                  flags|=YNegative;
+                  double_val=ceil(-double_val-0.5);
+                  if (MagickLongRangeOk(double_val))
+                    {
+                      bounds.y=(long) ceil(double_val);
+                      flags|=YValue;
+                      flags|=YNegative;
+                    }
                 }
             }
-          if (count)
-            flags|=YValue;
         }
     }
   if (*p != '\0')
diff -r 27a561878992 -r 90a7c0b86df3 www/Changelog.html
--- a/www/Changelog.html	Sat Apr 08 10:14:29 2023 -0500
+++ b/www/Changelog.html	Sat Apr 08 12:14:35 2023 -0500
@@ -40,6 +40,9 @@
 <p>2023-04-08  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>magick/utility.c (GetGeometry): Improve geometry parser to
+validate that parsed double values do not underflow or overflow
+when cast to 'unsigned long' or 'long' types.</p></li>
 <li><p>coders/mpc.c (ReadMPCImage): If an attribute appears multiple
 times in the MPC header, only set it once.</p></li>
 <li><p>coders/miff.c (ReadMIFFImage): If an attribute appears multiple