WritePCXImage_Fix_heap_overflow

# HG changeset patch
# User Bob Friesenhahn <bfriesen@GraphicsMagick.org>
# Date 1672347762 21600
# Node ID aed8f9cb12c18b424224a7c40748a5491b1d5e6a
# Parent  c2f340f172790967254819fbf5d5664564eeec80
WritePCXImage(): Fix heap overflow when writing more than 1023 scenes, and also eliminate use of uninitialized memory.

Gbp-Pq: Name WritePCXImage_Fix_heap_overflow.patch

diff --git a/ChangeLog b/ChangeLog
index 4d44be8cff5da49c0ccf193d165195117e7f529d..d7a0c97807ec3cd97803ffa69e8d0f736a1c2147 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2022-12-29  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
+
+       * coders/pcx.c (WritePCXImage): Fix heap overflow when writing
+       more than 1023 scenes, and also eliminate use of uninitialized
+       memory.
+
 2022-12-26  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
 
        * version.sh: Update shared library versioning in preparation for

diff --git a/coders/pcx.c b/coders/pcx.c
index f55ca88506bbb896df1548fbe20a5fd11c4aca03..fbd73cb59342c0adfce0854b1fef403eb41e5850 100644 (file)
--- a/coders/pcx.c
+++ b/coders/pcx.c
@@ -1029,6 +1029,9 @@ static unsigned int WritePCXImage(const ImageInfo *image_info,Image *image)
   unsigned long
     scene;
 
+  const unsigned long
+    max_scenes = 1024UL;
+
   ImageCharacteristics
     characteristics;
 
@@ -1057,11 +1060,12 @@ static unsigned int WritePCXImage(const ImageInfo *image_info,Image *image)
       */
       write_dcx=MagickTrue;
       (void) WriteBlobLSBLong(image,0x3ADE68B1L);
-      page_table=MagickAllocateResourceLimitedMemory(ExtendedSignedIntegralType *,
-        1024*sizeof(ExtendedSignedIntegralType));
+      page_table=MagickAllocateResourceLimitedClearedArray(ExtendedSignedIntegralType *,
+                                                           max_scenes+1,
+                                                           sizeof(ExtendedSignedIntegralType));
       if (page_table == (ExtendedSignedIntegralType *) NULL)
         ThrowPCXWriterException(ResourceLimitError,MemoryAllocationFailed,image);
-      for (scene=0; scene < 1024; scene++)
+      for (scene=0; scene < max_scenes; scene++)
         (void) WriteBlobLSBLong(image,0x00000000L);
     }
   adjoin=(image_info->adjoin) && (image->next != (const Image *) NULL) && (write_dcx);
@@ -1156,11 +1160,9 @@ static unsigned int WritePCXImage(const ImageInfo *image_info,Image *image)
     /*
       Dump colormap to file.
     */
-    pcx_colormap=MagickAllocateResourceLimitedMemory(unsigned char *,3*256);
+    pcx_colormap=MagickAllocateResourceLimitedClearedArray(unsigned char *,3,256);
     if (pcx_colormap == (unsigned char *) NULL)
       ThrowPCXWriterException(ResourceLimitError,MemoryAllocationFailed,image);
-    for (i=0; i < (3*256); i++)
-      pcx_colormap[i]=0;
     q=pcx_colormap;
     if (image->storage_class == PseudoClass)
       for (i=0; i < (long) image->colors; i++)
@@ -1177,7 +1179,9 @@ static unsigned int WritePCXImage(const ImageInfo *image_info,Image *image)
     for (i=0; i < 58; i++)
       (void) WriteBlobByte(image,'\0');
     /* Allocate memory for one pixel row. */
-    pcx_pixels=MagickAllocateResourceLimitedArray(unsigned char *,bytes_per_line,pcx_info.planes);
+    pcx_pixels=MagickAllocateResourceLimitedClearedArray(unsigned char *,
+                                                         bytes_per_line,
+                                                         pcx_info.planes);
     if (pcx_pixels == (unsigned char *) NULL)
       ThrowPCXWriterException(ResourceLimitError,MemoryAllocationFailed,image);
     q=pcx_pixels;
@@ -1329,12 +1333,12 @@ static unsigned int WritePCXImage(const ImageInfo *image_info,Image *image)
     if (image->next == (Image *) NULL)
       break;
     image=SyncNextImageInList(image);
-    status=MagickMonitorFormatted(scene++,image_list_length,
+    status=MagickMonitorFormatted(scene++,Min(max_scenes,image_list_length),
                                   &image->exception,SaveImagesText,
                                   image->filename);
     if (status == False)
       break;
-    if (scene >= 1023)
+    if (scene >= max_scenes-1)
       break;
   } while (adjoin);
   if (adjoin)
@@ -1345,6 +1349,10 @@ static unsigned int WritePCXImage(const ImageInfo *image_info,Image *image)
       /*
         Write the DCX page table.
       */
+      if (logging && write_dcx && image_list_length > max_scenes)
+        (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                              "WARNING: DCX truncated to %lu scenes!",
+                              max_scenes-1);
       page_table[scene+1]=0;
       (void) SeekBlob(image,0L,SEEK_SET);
       (void) WriteBlobLSBLong(image,0x3ADE68B1L);

diff --git a/www/Changelog.html b/www/Changelog.html
index 04d3798629dfba6d93951de9cbf76975e927c5c1..91abba8c1e95bc052f163067314948c791df61ec 100644 (file)
--- a/www/Changelog.html
+++ b/www/Changelog.html
@@ -37,6 +37,14 @@
 </div>
 
 <div class="document">
+<p>2022-12-29  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/pcx.c (WritePCXImage): Fix heap overflow when writing
+more than 1023 scenes, and also eliminate use of uninitialized
+memory.</p></li>
+</ul>
+</blockquote>
 <p>2022-12-26  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">