GraphicsMagick: WriteOnePNGImage(): Address undefined behavior w...

GraphicsMagick Commits <[email protected]>
Newsgroups gmane.comp.video.graphicsmagick.cvs
Message-ID <mailman.4614.1680981562.1789.graphicsmagick-commit@lists.sourceforge.net> 
changeset f462cca796b5 in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=f462cca796b5
summary: WriteOnePNGImage(): Address undefined behavior while converting floating point resolution to unsigned integer.

diffstat:

 ChangeLog                |    7 +-
 coders/png.c             |   16 +-
 magick/utility-private.h |    7 +-
 magick/utility.c         |  215 +++++++++++++++++++++++++++++++++++++++++++++++
 www/Changelog.html       |    6 +-
 5 files changed, 240 insertions(+), 11 deletions(-)

diffs (326 lines):

diff -r 90a7c0b86df3 -r f462cca796b5 ChangeLog
--- a/ChangeLog	Sat Apr 08 12:14:35 2023 -0500
+++ b/ChangeLog	Sat Apr 08 14:19:11 2023 -0500
@@ -1,8 +1,13 @@
 2023-04-08  Bob Friesenhahn  <[email protected]>
 
+	* coders/png.c (WriteOnePNGImage): Address undefined behavior
+	while converting floating point resolution to unsigned integer.
+	(SourceForge issue #706 test case 'bug19');
+
 	* 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.
+	when cast to 'unsigned long' or 'long' types. (SourceForge issue
+	#706 test case 'bug11');
 
 	* coders/mpc.c (ReadMPCImage): If an attribute appears multiple
 	times in the MPC header, only set it once.
diff -r 90a7c0b86df3 -r f462cca796b5 coders/png.c
--- a/coders/png.c	Sat Apr 08 12:14:35 2023 -0500
+++ b/coders/png.c	Sat Apr 08 14:19:11 2023 -0500
@@ -8140,20 +8140,20 @@
       if (image->units == PixelsPerInchResolution)
         {
           unit_type=PNG_RESOLUTION_METER;
-          x_resolution=(png_uint_32) ((100.0*image->x_resolution+0.5)/2.54);
-          y_resolution=(png_uint_32) ((100.0*image->y_resolution+0.5)/2.54);
+          x_resolution=(png_uint_32) MagickDoubleToUInt((100.0*image->x_resolution+0.5)/2.54);
+          y_resolution=(png_uint_32) MagickDoubleToUInt((100.0*image->y_resolution+0.5)/2.54);
         }
       else if (image->units == PixelsPerCentimeterResolution)
         {
           unit_type=PNG_RESOLUTION_METER;
-          x_resolution=(png_uint_32) (100.0*image->x_resolution+0.5);
-          y_resolution=(png_uint_32) (100.0*image->y_resolution+0.5);
+          x_resolution=(png_uint_32) MagickDoubleToUInt(100.0*image->x_resolution+0.5);
+          y_resolution=(png_uint_32) MagickDoubleToUInt(100.0*image->y_resolution+0.5);
         }
       else
         {
           unit_type=PNG_RESOLUTION_UNKNOWN;
-          x_resolution=(png_uint_32) image->x_resolution;
-          y_resolution=(png_uint_32) image->y_resolution;
+          x_resolution=(png_uint_32) MagickDoubleToUInt(image->x_resolution);
+          y_resolution=(png_uint_32) MagickDoubleToUInt(image->y_resolution);
         }
 
       png_set_pHYs(ping,ping_info,x_resolution,y_resolution,unit_type);
@@ -8163,9 +8163,9 @@
         (void) LogMagickEvent(CoderEvent,GetMagickModule(),
              "    Setting up pHYs chunk");
         (void) LogMagickEvent(CoderEvent,GetMagickModule(),
-             "      x_resolution=%lu",(unsigned long) x_resolution);
+             "      x_resolution=%u",(unsigned int) x_resolution);
         (void) LogMagickEvent(CoderEvent,GetMagickModule(),
-             "      y_resolution=%lu",(unsigned long) y_resolution);
+             "      y_resolution=%u",(unsigned int) y_resolution);
         (void) LogMagickEvent(CoderEvent,GetMagickModule(),
              "      unit_type=%lu",(unsigned long) unit_type);
       }
diff -r 90a7c0b86df3 -r f462cca796b5 magick/utility-private.h
--- a/magick/utility-private.h	Sat Apr 08 12:14:35 2023 -0500
+++ b/magick/utility-private.h	Sat Apr 08 14:19:11 2023 -0500
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2003 - 2020 GraphicsMagick Group
+  Copyright (C) 2003 - 2023 GraphicsMagick Group
   Copyright (C) 2002 ImageMagick Studio
   Copyright 1991-1999 E. I. du Pont de Nemours and Company
 
@@ -31,7 +31,12 @@
 extern MagickExport MagickPassFail MagickAtoUIChk(const char *str, unsigned int *value);
 extern MagickExport MagickPassFail MagickAtoLChk(const char *str, long *value);
 extern MagickExport MagickPassFail MagickAtoULChk(const char *str, unsigned long *value);
+extern MagickExport unsigned long MagickDoubleToULong(const double dval/*, ExceptionInfo *exception*/) MAGICK_FUNC_CONST;
+extern MagickExport unsigned int MagickDoubleToUInt(const double dval/*, ExceptionInfo *exception*/) MAGICK_FUNC_CONST;
 extern MagickExport long MagickDoubleToLong(const double dval/*, ExceptionInfo *exception*/) MAGICK_FUNC_CONST;
+extern MagickExport int MagickDoubleToInt(const double dval/*, ExceptionInfo *exception*/) MAGICK_FUNC_CONST;
+extern MagickExport unsigned short int MagickDoubleToUShort(const double dval/*, ExceptionInfo *exception*/) MAGICK_FUNC_CONST;
+extern MagickExport short int MagickDoubleToShort(const double dval/*, ExceptionInfo *exception*/) MAGICK_FUNC_CONST;
 
 extern MagickExport size_t
   MagickStripSpacesFromString(char *string),
diff -r 90a7c0b86df3 -r f462cca796b5 magick/utility.c
--- a/magick/utility.c	Sat Apr 08 12:14:35 2023 -0500
+++ b/magick/utility.c	Sat Apr 08 14:19:11 2023 -0500
@@ -3714,6 +3714,221 @@
 }
 
 /*
+  Convert a double to an int, with clipping.
+  Someday a warning or an error may be produced here.
+*/
+MagickExport int MagickDoubleToInt(const double dval/*, ExceptionInfo *exception*/)
+{
+  int lval;
+
+  do
+    {
+#if defined(INFINITY)
+      if (dval == +INFINITY)
+        {
+          lval=INT_MAX;
+          break;
+        }
+      if (dval == -INFINITY)
+        {
+          lval=INT_MIN;
+          break;
+        }
+#endif
+      if (isnan(dval))
+        {
+          lval=0;
+          break;
+        }
+      if (floor(dval) > ((double) INT_MAX - 1))
+        {
+          lval=INT_MAX;
+          break;
+        }
+      if (ceil(dval) < ((double) INT_MIN + 1))
+        {
+          lval=INT_MIN;
+          break;
+        }
+      lval=(int) dval;
+    } while (0);
+
+  return lval;
+}
+
+/*
+  Convert a double to an unsigned long, with clipping.
+  Someday a warning or an error may be produced here.
+*/
+MagickExport unsigned long MagickDoubleToULong(const double dval/*, ExceptionInfo *exception*/)
+{
+  unsigned long lval;
+
+  do
+    {
+#if defined(INFINITY)
+      if (dval == +INFINITY)
+        {
+          lval=ULONG_MAX;
+          break;
+        }
+      if (dval == -INFINITY)
+        {
+          lval=0;
+          break;
+        }
+#endif
+      if (isnan(dval))
+        {
+          lval=0;
+          break;
+        }
+      if (floor(dval) > ((double) ULONG_MAX - 1))
+        {
+          lval=ULONG_MAX;
+          break;
+        }
+      if (ceil(dval) < 0.0)
+        {
+          lval=0;
+          break;
+        }
+      lval=(unsigned long) dval;
+    } while (0);
+
+  return lval;
+}
+
+/*
+  Convert a double to an unsigned int, with clipping.
+  Someday a warning or an error may be produced here.
+*/
+MagickExport unsigned int MagickDoubleToUInt(const double dval/*, ExceptionInfo *exception*/)
+{
+  unsigned int lval;
+
+  do
+    {
+#if defined(INFINITY)
+      if (dval == +INFINITY)
+        {
+          lval=UINT_MAX;
+          break;
+        }
+      if (dval == -INFINITY)
+        {
+          lval=0;
+          break;
+        }
+#endif
+      if (isnan(dval))
+        {
+          lval=0;
+          break;
+        }
+      if (floor(dval) > ((double) UINT_MAX - 1))
+        {
+          lval=UINT_MAX;
+          break;
+        }
+      if (ceil(dval) < 0.0)
+        {
+          lval=0;
+          break;
+        }
+      lval=(unsigned int) dval;
+    } while (0);
+
+  return lval;
+}
+
+/*
+  Convert a double to a short, with clipping.
+  Someday a warning or an error may be produced here.
+*/
+MagickExport short int MagickDoubleToShort(const double dval/*, ExceptionInfo *exception*/)
+{
+  short int lval;
+
+  do
+    {
+#if defined(INFINITY)
+      if (dval == +INFINITY)
+        {
+          lval=SHRT_MAX;
+          break;
+        }
+      if (dval == -INFINITY)
+        {
+          lval=SHRT_MIN;
+          break;
+        }
+#endif
+      if (isnan(dval))
+        {
+          lval=0;
+          break;
+        }
+      if (floor(dval) > ((double) SHRT_MAX - 1))
+        {
+          lval=SHRT_MAX;
+          break;
+        }
+      if (ceil(dval) < ((double) SHRT_MIN + 1))
+        {
+          lval=SHRT_MIN;
+          break;
+        }
+      lval=(short int) dval;
+    } while (0);
+
+  return lval;
+}
+
+/*
+  Convert a double to an unsigned short, with clipping.
+  Someday a warning or an error may be produced here.
+*/
+MagickExport unsigned short int MagickDoubleToUShort(const double dval/*, ExceptionInfo *exception*/)
+{
+  unsigned short int lval;
+
+  do
+    {
+#if defined(INFINITY)
+      if (dval == +INFINITY)
+        {
+          lval=USHRT_MAX;
+          break;
+        }
+      if (dval == -INFINITY)
+        {
+          lval=0;
+          break;
+        }
+#endif
+      if (isnan(dval))
+        {
+          lval=0;
+          break;
+        }
+      if (floor(dval) > ((double) USHRT_MAX - 1))
+        {
+          lval=USHRT_MAX;
+          break;
+        }
+      if (ceil(dval) < 0.0)
+        {
+          lval=0;
+          break;
+        }
+      lval=(unsigned short int) dval;
+    } while (0);
+
+  return lval;
+}
+
+/*
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %                                                                             %
 %                                                                             %
diff -r 90a7c0b86df3 -r f462cca796b5 www/Changelog.html
--- a/www/Changelog.html	Sat Apr 08 12:14:35 2023 -0500
+++ b/www/Changelog.html	Sat Apr 08 14:19:11 2023 -0500
@@ -40,9 +40,13 @@
 <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>coders/png.c (WriteOnePNGImage): Address undefined behavior
+while converting floating point resolution to unsigned integer.
+(SourceForge issue #706 test case 'bug19');</p></li>
 <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>
+when cast to 'unsigned long' or 'long' types. (SourceForge issue
+#706 test case 'bug11');</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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.