[libpng16] Moved chunk-length check into a png_check_chunk_length() p…

…rivate function (Suggested by Max Stepin).
MoHaoNan · Aug 4, 2017 · 2dca156 · 2dca156
1 parent 469317d
commit 2dca156
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 66 deletions.
diff --git a/ANNOUNCE b/ANNOUNCE
@@ -1,4 +1,4 @@
-Libpng 1.6.32beta10 - August 3, 2017
+Libpng 1.6.32beta10 - August 4, 2017
 
 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.
@@ -69,7 +69,9 @@ Version 1.6.32beta09 [August 3, 2017]
     no longer using deprecated cmake LOCATION feature (Clifford Yapp).
   Fixed five-byte error in the calculation of IDAT maximum possible size.
 
-Version 1.6.32beta10 [August 3, 2017]
+Version 1.6.32beta10 [August 4, 2017]
+  Moved chunk-length check into a png_check_chunk_length() private
+    function (Suggested by Max Stepin).
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

diff --git a/CHANGES b/CHANGES
@@ -5952,7 +5952,9 @@ Version 1.6.32beta09 [August 3, 2017]
     no longer using deprecated cmake LOCATION feature (Clifford Yapp).
   Fixed five-byte error in the calculation of IDAT maximum possible size.
 
-Version 1.6.32beta10 [August 3, 2017]
+Version 1.6.32beta10 [August 4, 2017]
+  Moved chunk-length check into a png_check_chunk_length() private
+    function (Suggested by Max Stepin).
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

diff --git a/pngpread.c b/pngpread.c
@@ -190,6 +190,8 @@ png_push_read_chunk(png_structrp png_ptr, png_inforp info_ptr)
       png_crc_read(png_ptr, chunk_tag, 4);
       png_ptr->chunk_name = PNG_CHUNK_FROM_STRING(chunk_tag);
       png_check_chunk_name(png_ptr, png_ptr->chunk_name);
+      png_check_chunk_length(png_ptr, png_ptr->chunk_name,
+          png_ptr->push_length);
       png_ptr->mode |= PNG_HAVE_CHUNK_HEADER;
    }
 
@@ -224,38 +226,6 @@ png_push_read_chunk(png_structrp png_ptr, png_inforp info_ptr)
          png_benign_error(png_ptr, "Too many IDATs found");
    }
 
-   if (chunk_name == png_IDAT)
-   {
-      size_t row_factor =
-         (png_ptr->width * png_ptr->channels * (png_ptr->bit_depth > 8? 2: 1)
-          + 1 + (png_ptr->interlaced? 6: 0));
-      if (png_ptr->height > PNG_UINT_32_MAX/row_factor)
-         limit=PNG_UINT_31_MAX;
-      else
-         limit = png_ptr->height * row_factor;
-      limit += 6 + 5*(limit/32566+1); /* zlib+deflate overhead */
-      limit=limit < PNG_UINT_31_MAX? limit : PNG_UINT_31_MAX;
-   }
-   else
-   {
-# ifdef PNG_SET_USER_LIMITS_SUPPORTED
-      if (png_ptr->user_chunk_malloc_max > 0 &&
-          png_ptr->user_chunk_malloc_max < limit)
-         limit = png_ptr->user_chunk_malloc_max;
-# elif PNG_USER_CHUNK_MALLOC_MAX > 0
-      if (PNG_USER_CHUNK_MALLOC_MAX < limit)
-         limit = PNG_USER_CHUNK_MALLOC_MAX;
-# endif
-   }
-   if (png_ptr->push_length > limit)
-   {
-      printf(" png_ptr->push_length = %lu, limit = %lu\n",
-         (unsigned long)png_ptr->push_length,(unsigned long)limit);
-      png_debug2(0," png_ptr->push_length = %lu, limit = %lu",
-         (unsigned long)png_ptr->push_length,(unsigned long)limit);
-      png_chunk_error(png_ptr, "chunk data is too large");
-   }
-
    if (chunk_name == png_IHDR)
    {
       if (png_ptr->push_length != 13)

diff --git a/pngpriv.h b/pngpriv.h
@@ -1530,6 +1530,9 @@ PNG_INTERNAL_FUNCTION(void,png_handle_zTXt,(png_structrp png_ptr,
 PNG_INTERNAL_FUNCTION(void,png_check_chunk_name,(png_structrp png_ptr,
     png_uint_32 chunk_name),PNG_EMPTY);
 
+PNG_INTERNAL_FUNCTION(void,png_check_chunk_length,(png_structrp png_ptr,
+    png_uint_32 chunk_name, png_uint_32 chunk_length),PNG_EMPTY);
+
 PNG_INTERNAL_FUNCTION(void,png_handle_unknown,(png_structrp png_ptr,
     png_inforp info_ptr, png_uint_32 length, int keep),PNG_EMPTY);
    /* This is the function that gets called for unknown chunks.  The 'keep'

diff --git a/pngrutil.c b/pngrutil.c
@@ -157,7 +157,6 @@ png_read_chunk_header(png_structrp png_ptr)
 {
    png_byte buf[8];
    png_uint_32 length;
-   png_alloc_size_t limit = PNG_UINT_31_MAX;
 
 #ifdef PNG_IO_STATE_SUPPORTED
    png_ptr->io_state = PNG_IO_READING | PNG_IO_CHUNK_HDR;
@@ -183,36 +182,7 @@ png_read_chunk_header(png_structrp png_ptr)
    png_check_chunk_name(png_ptr, png_ptr->chunk_name);
 
    /* Check for too-large chunk length */
-   if (png_ptr->chunk_name != png_IDAT)
-   {
-# ifdef PNG_SET_USER_LIMITS_SUPPORTED
-      if (png_ptr->user_chunk_malloc_max > 0 &&
-          png_ptr->user_chunk_malloc_max < limit)
-         limit = png_ptr->user_chunk_malloc_max;
-# elif PNG_USER_CHUNK_MALLOC_MAX > 0
-      if (PNG_USER_CHUNK_MALLOC_MAX < limit)
-         limit = PNG_USER_CHUNK_MALLOC_MAX;
-# endif
-   }
-   else
-   {
-      size_t row_factor =
-         (png_ptr->width * png_ptr->channels * (png_ptr->bit_depth > 8? 2: 1)
-          + 1 + (png_ptr->interlaced? 6: 0));
-      if (png_ptr->height > PNG_UINT_32_MAX/row_factor)
-         limit=PNG_UINT_31_MAX;
-      else
-         limit = png_ptr->height * row_factor;
-      limit += 6 + 5*(limit/32566+1); /* zlib+deflate overhead */
-      limit=limit < PNG_UINT_31_MAX? limit : PNG_UINT_31_MAX;
-   }
-
-   if (length > limit)
-   {
-      png_debug2(0," length = %lu, limit = %lu",
-         (unsigned long)length,(unsigned long)limit);
-      png_chunk_error(png_ptr, "chunk data is too large");
-   }
+   png_check_chunk_length(png_ptr, png_ptr->chunk_name, length);
 
 #ifdef PNG_IO_STATE_SUPPORTED
    png_ptr->io_state = PNG_IO_READING | PNG_IO_CHUNK_DATA;
@@ -3134,6 +3104,44 @@ png_check_chunk_name(png_structrp png_ptr, png_uint_32 chunk_name)
    }
 }
 
+void /* PRIVATE */
+png_check_chunk_length(png_structrp png_ptr, png_uint_32 chunk_name,
+   png_uint_32 length)
+{
+   png_alloc_size_t limit = PNG_UINT_31_MAX;
+
+   if (png_ptr->chunk_name != png_IDAT)
+   {
+# ifdef PNG_SET_USER_LIMITS_SUPPORTED
+      if (png_ptr->user_chunk_malloc_max > 0 &&
+          png_ptr->user_chunk_malloc_max < limit)
+         limit = png_ptr->user_chunk_malloc_max;
+# elif PNG_USER_CHUNK_MALLOC_MAX > 0
+      if (PNG_USER_CHUNK_MALLOC_MAX < limit)
+         limit = PNG_USER_CHUNK_MALLOC_MAX;
+# endif
+   }
+   else
+   {
+      size_t row_factor =
+         (png_ptr->width * png_ptr->channels * (png_ptr->bit_depth > 8? 2: 1)
+          + 1 + (png_ptr->interlaced? 6: 0));
+      if (png_ptr->height > PNG_UINT_32_MAX/row_factor)
+         limit=PNG_UINT_31_MAX;
+      else
+         limit = png_ptr->height * row_factor;
+      limit += 6 + 5*(limit/32566+1); /* zlib+deflate overhead */
+      limit=limit < PNG_UINT_31_MAX? limit : PNG_UINT_31_MAX;
+   }
+
+   if (length > limit)
+   {
+      png_debug2(0," length = %lu, limit = %lu",
+         (unsigned long)length,(unsigned long)limit);
+      png_chunk_error(png_ptr, "chunk data is too large");
+   }
+}
+
 /* Combines the row recently read in with the existing pixels in the row.  This
  * routine takes care of alpha and transparency if requested.  This routine also
  * handles the two methods of progressive display of interlaced images,