[libpng16] Reject oversized iCCP profile length

The code now validates the ICC profile length against the user chunk limit
before the buffer is allocated, as opposed to doing it while the buffer is read.

This removes the potential to consume virtual address space with a carefully
crafted ICC profile; only an issue on 32-bit systems where a valid profile can
be up to 2^32-4 bytes in length.  libpng never writes beyond the application
supplied limit, but previously it did allocate a buffer of the size specified in
the profile header.  The exploitability of this is almost zero; the address
space is released as soon as the PNG read completes.

Also clean up PNG_DEBUG compile of pngtest.c.

Signed-off-by: John Bowler <jbowler@acm.org>

ANNOUNCE

@@ -1,4 +1,4 @@
-Libpng 1.6.25beta01 - August 4, 2016
+Libpng 1.6.25beta01 - August 10, 2016
 
 This is not intended to be a public release.  It will be replaced
 within a few weeks by a public version or by another test version.
@@ -25,7 +25,9 @@ Other information:
 
 Changes since the last public release (1.6.24):
 
-Version 1.6.25beta01 [August 4, 2016]
+Version 1.6.25beta01 [August 10, 2016]
+  Reject oversized iCCP profile immediately.
+  Clean up PNG_DEBUG compile of pngtest.c.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

CHANGES

@@ -5675,7 +5675,9 @@ Version 1.6.24rc03 [August 2, 2016]
 Version 1.6.24[August 4, 2016]
   No changes.
 
-Version 1.6.24[August 4, 2016]
+Version 1.6.25beta01 [August 10, 2016]
+  Reject oversized iCCP profile immediately.
+  Clean up PNG_DEBUG compile of pngtest.c.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

png.c

@@ -1931,8 +1931,8 @@ png_colorspace_set_sRGB(png_const_structrp png_ptr, png_colorspacerp colorspace,
 static const png_byte D50_nCIEXYZ[12] =
    { 0x00, 0x00, 0xf6, 0xd6, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0xd3, 0x2d };
 
-int /* PRIVATE */
+static int /* bool */
-png_icc_check_length(png_const_structrp png_ptr, png_colorspacerp colorspace,
+icc_check_length(png_const_structrp png_ptr, png_colorspacerp colorspace,
     png_const_charp name, png_uint_32 profile_length)
 {
    if (profile_length < 132)
@@ -1942,6 +1942,40 @@ png_icc_check_length(png_const_structrp png_ptr, png_colorspacerp colorspace,
    return 1;
 }
 
+#ifdef PNG_READ_iCCP_SUPPORTED
+int /* PRIVATE */
+png_icc_check_length(png_const_structrp png_ptr, png_colorspacerp colorspace,
+    png_const_charp name, png_uint_32 profile_length)
+{
+   if (!icc_check_length(png_ptr, colorspace, name, profile_length))
+      return 0;
+
+   /* This needs to be here because the 'normal' check is in
+    * png_decompress_chunk, yet this happens after the attempt to
+    * png_malloc_base the required data.  We only need this on read; on write
+    * the caller supplies the profile buffer so libpng doesn't allocate it.  See
+    * the call to icc_check_length below (the write case).
+    */
+#  ifdef PNG_SET_USER_LIMITS_SUPPORTED
+      else if (png_ptr->user_chunk_malloc_max > 0 &&
+               png_ptr->user_chunk_malloc_max < profile_length)
+         return png_icc_profile_error(png_ptr, colorspace, name, profile_length,
+             "exceeds application limits");
+#  elif PNG_USER_CHUNK_MALLOC_MAX > 0
+      else if (PNG_USER_CHUNK_MALLOC_MAX < profile_length)
+         return png_icc_profile_error(png_ptr, colorspace, name, profile_length,
+             "exceeds libpng limits");
+#  else /* !SET_USER_LIMITS */
+      /* This will get compiled out on all 32-bit and better systems. */
+      else if (PNG_SIZE_MAX < profile_length)
+         return png_icc_profile_error(png_ptr, colorspace, name, profile_length,
+             "exceeds system limits");
+#  endif /* !SET_USER_LIMITS */
+
+   return 1;
+}
+#endif /* READ_iCCP */
+
 int /* PRIVATE */
 png_icc_check_header(png_const_structrp png_ptr, png_colorspacerp colorspace,
     png_const_charp name, png_uint_32 profile_length,
@@ -2377,7 +2411,7 @@ png_colorspace_set_ICC(png_const_structrp png_ptr, png_colorspacerp colorspace,
    if ((colorspace->flags & PNG_COLORSPACE_INVALID) != 0)
       return 0;
 
-   if (png_icc_check_length(png_ptr, colorspace, name, profile_length) != 0 &&
+   if (icc_check_length(png_ptr, colorspace, name, profile_length) != 0 &&
        png_icc_check_header(png_ptr, colorspace, name, profile_length, profile,
            color_type) != 0 &&
        png_icc_check_tag_table(png_ptr, colorspace, name, profile_length,

pngpriv.h

@@ -1494,9 +1494,11 @@ PNG_INTERNAL_FUNCTION(int,png_colorspace_set_ICC,(png_const_structrp png_ptr,
    /* The 'name' is used for information only */
 
 /* Routines for checking parts of an ICC profile. */
+#ifdef PNG_READ_iCCP_SUPPORTED
 PNG_INTERNAL_FUNCTION(int,png_icc_check_length,(png_const_structrp png_ptr,
    png_colorspacerp colorspace, png_const_charp name,
    png_uint_32 profile_length), PNG_EMPTY);
+#endif /* READ_iCCP */
 PNG_INTERNAL_FUNCTION(int,png_icc_check_header,(png_const_structrp png_ptr,
    png_colorspacerp colorspace, png_const_charp name,
    png_uint_32 profile_length,

pngtest.c

@@ -514,10 +514,10 @@ typedef struct memory_information
 typedef memory_information *memory_infop;
 
 static memory_infop pinformation = NULL;
-static int current_allocation = 0;
+static png_alloc_size_t current_allocation = 0;
-static int maximum_allocation = 0;
+static png_alloc_size_t maximum_allocation = 0;
-static int total_allocation = 0;
+static png_alloc_size_t total_allocation = 0;
-static int num_allocations = 0;
+static png_alloc_size_t num_allocations = 0;
 
 png_voidp PNGCBAPI png_debug_malloc PNGARG((png_structp png_ptr,
     png_alloc_size_t size));
@@ -604,9 +604,10 @@ png_debug_free(png_structp png_ptr, png_voidp ptr)
          if (pinfo->pointer == ptr)
          {
             *ppinfo = pinfo->next;
-            current_allocation -= pinfo->size;
+            if (current_allocation < pinfo->size)
-            if (current_allocation < 0)
                fprintf(STDERR, "Duplicate free of memory\n");
+            else
+               current_allocation -= pinfo->size;
             /* We must free the list element too, but first kill
                the memory that is to be freed. */
             memset(ptr, 0x55, pinfo->size);
@@ -938,6 +939,12 @@ test_one_file(PNG_CONST char *inname, PNG_CONST char *outname)
        read_user_chunk_callback);
 #endif
 
+#ifdef PNG_SET_USER_LIMITS_SUPPORTED
+#  ifdef CHUNK_LIMIT /* from the build, for testing */
+      png_set_chunk_malloc_max(read_ptr, CHUNK_LIMIT);
+#  endif /* CHUNK_LIMIT */
+#endif
+
 #ifdef PNG_SETJMP_SUPPORTED
    pngtest_debug("Setting jmpbuf for read struct");
    if (setjmp(png_jmpbuf(read_ptr)))
@@ -1876,7 +1883,7 @@ main(int argc, char *argv[])
    {
       int i;
 #if defined(PNG_USER_MEM_SUPPORTED) && PNG_DEBUG
-      int allocation_now = current_allocation;
+      png_alloc_size_t allocation_now = current_allocation;
 #endif
       for (i=2; i<argc; ++i)
       {
@@ -1909,15 +1916,15 @@ main(int argc, char *argv[])
          }
 #if defined(PNG_USER_MEM_SUPPORTED) && PNG_DEBUG
          if (allocation_now != current_allocation)
-            fprintf(STDERR, "MEMORY ERROR: %d bytes lost\n",
+            fprintf(STDERR, "MEMORY ERROR: %lu bytes lost\n",
-                current_allocation - allocation_now);
+                (unsigned long)(current_allocation - allocation_now));
 
          if (current_allocation != 0)
          {
             memory_infop pinfo = pinformation;
 
-            fprintf(STDERR, "MEMORY ERROR: %d bytes still allocated\n",
+            fprintf(STDERR, "MEMORY ERROR: %lu bytes still allocated\n",
-                current_allocation);
+                (unsigned long)current_allocation);
 
             while (pinfo != NULL)
             {
@@ -1930,14 +1937,14 @@ main(int argc, char *argv[])
 #endif
       }
 #if defined(PNG_USER_MEM_SUPPORTED) && PNG_DEBUG
-         fprintf(STDERR, " Current memory allocation: %10d bytes\n",
+         fprintf(STDERR, " Current memory allocation: %20lu bytes\n",
-             current_allocation);
+             (unsigned long)current_allocation);
-         fprintf(STDERR, " Maximum memory allocation: %10d bytes\n",
+         fprintf(STDERR, " Maximum memory allocation: %20lu bytes\n",
-             maximum_allocation);
+             (unsigned long) maximum_allocation);
-         fprintf(STDERR, " Total   memory allocation: %10d bytes\n",
+         fprintf(STDERR, " Total   memory allocation: %20lu bytes\n",
-             total_allocation);
+             (unsigned long)total_allocation);
-         fprintf(STDERR, "     Number of allocations: %10d\n",
+         fprintf(STDERR, "     Number of allocations: %20lu\n",
-             num_allocations);
+             (unsigned long)num_allocations);
 #endif
    }
 
@@ -1948,7 +1955,7 @@ main(int argc, char *argv[])
       {
          int kerror;
 #if defined(PNG_USER_MEM_SUPPORTED) && PNG_DEBUG
-         int allocation_now = current_allocation;
+         png_alloc_size_t allocation_now = current_allocation;
 #endif
          if (i == 1)
             status_dots_requested = 1;
@@ -1998,15 +2005,15 @@ main(int argc, char *argv[])
          }
 #if defined(PNG_USER_MEM_SUPPORTED) && PNG_DEBUG
          if (allocation_now != current_allocation)
-             fprintf(STDERR, "MEMORY ERROR: %d bytes lost\n",
+             fprintf(STDERR, "MEMORY ERROR: %lu bytes lost\n",
-                 current_allocation - allocation_now);
+                 (unsigned long)(current_allocation - allocation_now));
 
          if (current_allocation != 0)
          {
              memory_infop pinfo = pinformation;
 
-             fprintf(STDERR, "MEMORY ERROR: %d bytes still allocated\n",
+             fprintf(STDERR, "MEMORY ERROR: %lu bytes still allocated\n",
-                 current_allocation);
+                 (unsigned long)current_allocation);
 
              while (pinfo != NULL)
              {
@@ -2018,14 +2025,14 @@ main(int argc, char *argv[])
 #endif
        }
 #if defined(PNG_USER_MEM_SUPPORTED) && PNG_DEBUG
-       fprintf(STDERR, " Current memory allocation: %10d bytes\n",
+       fprintf(STDERR, " Current memory allocation: %20lu bytes\n",
-           current_allocation);
+           (unsigned long)current_allocation);
-       fprintf(STDERR, " Maximum memory allocation: %10d bytes\n",
+       fprintf(STDERR, " Maximum memory allocation: %20lu bytes\n",
-           maximum_allocation);
+           (unsigned long)maximum_allocation);
-       fprintf(STDERR, " Total   memory allocation: %10d bytes\n",
+       fprintf(STDERR, " Total   memory allocation: %20lu bytes\n",
-           total_allocation);
+           (unsigned long)total_allocation);
-       fprintf(STDERR, "     Number of allocations: %10d\n",
+       fprintf(STDERR, "     Number of allocations: %20lu\n",
-           num_allocations);
+           (unsigned long)num_allocations);
 #endif
    }