fix_bounds_issue_when_concatenating_string

# HG changeset patch
# User Bob Friesenhahn <bfriesen@GraphicsMagick.org>
# Date 1680966869 18000
# Node ID 27a561878992e8588a9c80f3fce51e66e0b55ebc
# Parent  5509b7e1b29b17b823d6bfdcf7d1519092bf7d8a
Address issues from SourceForge issue #706 test case 'bug11'

Gbp-Pq: Name fix_bounds_issue_when_concatenating_string.patch

diff --git a/ChangeLog b/ChangeLog
index 0573f5329c36d72d161fb9eb9baa7af3c80ec129..03aa5ae8e6ee55a0c6814ba70a005470ecf907ee 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2023-04-08  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
+
+       * coders/mpc.c (ReadMPCImage): If an attribute appears multiple
+       times in the MPC header, only set it once.
+
+       * coders/miff.c (ReadMIFFImage): If an attribute appears multiple
+       times in the MIFF header, only set it once.
+
+       * magick/attribute.c (SetImageAttribute): Fix bounds issue when
+       concatenating string (SourceForge issue #706 test case 'bug11');
+
 2023-01-14  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
 
        * version.sh: Updated for 1.3.40 release.

diff --git a/coders/miff.c b/coders/miff.c
index 5f6dca956d9723ea5cf6a9066d2fa3b1ec2c7f4b..f26b810c6bea952c2360051c2b0de63d2840f551 100644 (file)
--- a/coders/miff.c
+++ b/coders/miff.c
@@ -752,6 +752,23 @@ do { \
 
 #define ReadMIFFMaxKeyWordCount 256 /* Arbitrary limit on keywords in one MIFF frame */
 
+/*
+  Ignore attempts to set the same attribute multiple times.
+*/
+static MagickPassFail
+SetNewImageAttribute(Image *image,const char *key,const char *value)
+{
+  MagickPassFail
+    status;
+
+  if (GetImageAttribute(image,key) == (const ImageAttribute *) NULL)
+    status = SetImageAttribute(image,key,value);
+  else
+    status = MagickFail;
+
+  return status;
+};
+
 static Image *ReadMIFFImage(const ImageInfo *image_info,
   ExceptionInfo *exception)
 {
@@ -926,7 +943,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
               image);
           *p='\0';
           (void) LogMagickEvent(CoderEvent,GetMagickModule(),"Comment: \"%s\"", comment);
-          (void) SetImageAttribute(image,"comment",comment);
+          (void) SetNewImageAttribute(image,"comment",comment);
           comment_count++;
           MagickFreeResourceLimitedMemory(comment);
           c=ReadBlobByte(image);
@@ -1060,7 +1077,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                       exception);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1135,7 +1152,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     image->columns= MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1165,7 +1182,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                           image->dispose=PreviousDispose;
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1184,7 +1201,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                       &image->chromaticity.green_primary.y);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1201,7 +1218,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     image->iterations=MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1225,7 +1242,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     (void) CloneString(&image->montage,values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1243,7 +1260,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     image->orientation=StringToOrientationType(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1281,7 +1298,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     number_of_profiles++;
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1322,7 +1339,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     image->rows= MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1334,7 +1351,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     image->scene=MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1351,7 +1368,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                         image->units=PixelsPerCentimeterResolution;
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1363,7 +1380,7 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                     version=MagickAtoF(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -1377,13 +1394,13 @@ static Image *ReadMIFFImage(const ImageInfo *image_info,
                       &image->chromaticity.white_point.y);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
               default:
               {
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }

diff --git a/coders/mpc.c b/coders/mpc.c
index e554496812cb2128955d66bcde1c685e1f05de29..e98600d3195abd0efc3a738feaf671ce684275b9 100644 (file)
--- a/coders/mpc.c
+++ b/coders/mpc.c
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2022 GraphicsMagick Group
+% Copyright (C) 2003-2023 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 %
 % This program is covered by multiple licenses, which are described in
@@ -146,6 +146,23 @@ do { \
 
 #define ReadMPCMaxKeyWordCount 256 /* Arbitrary limit on number of keywords in MPC frame */
 
+/*
+  Ignore attempts to set the same attribute multiple times.
+*/
+static MagickPassFail
+SetNewImageAttribute(Image *image,const char *key,const char *value)
+{
+  MagickPassFail
+    status;
+
+  if (GetImageAttribute(image,key) == (const ImageAttribute *) NULL)
+    status = SetImageAttribute(image,key,value);
+  else
+    status = MagickFail;
+
+  return status;
+};
+
 static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
 {
   char
@@ -294,7 +311,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
             ThrowMPCReaderException(ResourceLimitError,MemoryAllocationFailed,
               image);
           *p='\0';
-          (void) SetImageAttribute(image,"comment",comment);
+          (void) SetNewImageAttribute(image,"comment",comment);
           comment_count++;
           MagickFreeResourceLimitedMemory(comment);
           c=ReadBlobByte(image);
@@ -429,7 +446,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                       exception);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -493,7 +510,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     image->columns= MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -523,7 +540,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                           image->dispose=PreviousDispose;
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -535,7 +552,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     image->error.mean_error_per_pixel=MagickAtoF(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -560,7 +577,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                       &image->chromaticity.green_primary.y);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -577,7 +594,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     image->iterations=MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -617,7 +634,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     (void) CloneString(&image->montage,values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -635,7 +652,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     image->orientation=StringToOrientationType(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -671,7 +688,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     number_of_profiles++;
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -683,7 +700,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     quantum_depth=MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -724,7 +741,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     image->rows=MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -736,7 +753,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                     image->scene=MagickAtoL(values);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -753,7 +770,7 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                         image->units=PixelsPerCentimeterResolution;
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
@@ -767,13 +784,13 @@ static Image *ReadMPCImage(const ImageInfo *image_info,ExceptionInfo *exception)
                       &image->chromaticity.white_point.y);
                     break;
                   }
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }
               default:
               {
-                (void) SetImageAttribute(image,keyword,
+                (void) SetNewImageAttribute(image,keyword,
                   *values == '{' ? values+1 : values);
                 break;
               }

diff --git a/magick/attribute.c b/magick/attribute.c
index ef814f480e7c6b2d87be5597d902576c95f7251d..d1768fb5423e64d342fbe7d8598779f17ad12d1e 100644 (file)
--- a/magick/attribute.c
+++ b/magick/attribute.c
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2022 GraphicsMagick Group
+% Copyright (C) 2003-2023 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 %
 % This program is covered by multiple licenses, which are described in
@@ -3294,15 +3294,18 @@ SetImageAttribute(Image *image,const char *key,const char *value)
           else
             {
               /*
-                Extend existing text string.
+                Extend existing text string.  This functionality is deprecated!
               */
               min_l=p->length+attribute->length+1;
               for (realloc_l=2; realloc_l <= min_l; realloc_l *= 2)
                     { /* nada */};
               MagickReallocMemory(char *,p->value,realloc_l);
               if (p->value != (char *) NULL)
-                (void) strlcat(p->value+p->length,attribute->value,min_l);
-              p->length += attribute->length;
+                {
+                  (void) memcpy(p->value+p->length,attribute->value,min_l-p->length-1);
+                  p->length += attribute->length;
+                  p->value[p->length] = '\0';
+                }
               DestroyImageAttribute(attribute);
             }
           if (p->value != (char *) NULL)

diff --git a/www/Changelog.html b/www/Changelog.html
index 3c86ad97e9e1993d1b8fdd66b9df33083040290f..e6abafff78bde3d73fccdd9b654d59985669412c 100644 (file)
--- a/www/Changelog.html
+++ b/www/Changelog.html
@@ -37,6 +37,17 @@
 </div>
 
 <div class="document">
+<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/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
+times in the MIFF header, only set it once.</p></li>
+<li><p>magick/attribute.c (SetImageAttribute): Fix bounds issue when
+concatenating string (SourceForge issue #706 test case 'bug11');</p></li>
+</ul>
+</blockquote>
 <p>2023-01-14  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">