[libpng16] Fixed conceivable but difficult to repro overflow. Also added

two test programs to generate and test a PNG which should have the problem.

ANNOUNCE

@@ -1,5 +1,5 @@
 
-Libpng 1.6.0beta37 - January 1, 2013
+Libpng 1.6.0beta37 - January 10, 2013
 
 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.
@@ -11,7 +11,6 @@ Source files with LF line endings (for Unix/Linux) and with a
 
    1.6.0beta37.tar.xz (LZMA-compressed, recommended)
    1.6.0beta37.tar.gz
-   1.6.0beta37.tar.bz2
 
 Source files with CRLF line endings (for Windows), without the
 "configure" script
@@ -584,7 +583,9 @@ Version 1.6.0beta36 [January 2, 2013]
   Rebuilt configure with autoconf-2.69 (inadvertently not done in beta33)
   Fixed 'make distcheck' on SUN OS - libpng.so was not being removed
 
-Version 1.6.0beta37 [January 1, 2013]
+Version 1.6.0beta37 [January 10, 2013]
+  Fixed conceivable but difficult to repro overflow. Also added two test
+    programs to generate and test a PNG which should have the problem.
 
   ===========================================================================
                        NOTICE November 17, 2012:

CHANGES

@@ -4336,7 +4336,9 @@ Version 1.6.0beta36 [January 2, 2013]
   Rebuilt configure with autoconf-2.69 (inadvertently not done in beta33)
   Fixed 'make distcheck' on SUN OS - libpng.so was not being removed
 
-Version 1.6.0beta37 [January 1, 2013]
+Version 1.6.0beta37 [January 10, 2013]
+  Fixed conceivable but difficult to repro overflow. Also added two test
+    programs to generate and test a PNG which should have the problem.
 
   ===========================================================================
                        NOTICE November 17, 2012:

contrib/libtests/fakepng.c

@@ -0,0 +1,57 @@
+/* Fake a PNG - just write it out directly. */
+#include <stdio.h>
+#include <zlib.h> /* for crc32 */
+
+void
+put_uLong(uLong val)
+{
+   putchar(val >> 24);
+   putchar(val >> 16);
+   putchar(val >>  8);
+   putchar(val >>  0);
+}
+
+void
+put_chunk(const unsigned char *chunk, uInt length)
+{
+   uLong crc;
+
+   put_uLong(length-4); /* Exclude the tag */
+
+   fwrite(chunk, length, 1, stdout);
+
+   crc = crc32(0, Z_NULL, 0);
+   put_uLong(crc32(crc, chunk, length));
+}
+
+const unsigned char signature[] =
+{
+   137, 80, 78, 71, 13, 10, 26, 10
+};
+
+const unsigned char IHDR[] =
+{
+   73, 72, 68, 82, /* IHDR */
+   0, 0, 0, 1, /* width */
+   0, 0, 0, 1, /* height */
+   1, /* bit depth */
+   0, /* color type: greyscale */
+   0, /* compression method */
+   0, /* filter method */
+   0  /* interlace method: none */
+};
+
+const unsigned char unknown[] =
+{
+   'u', 'n', 'K', 'n' /* "unKn" - private safe to copy */
+};
+
+int
+main(void)
+{
+   fwrite(signature, sizeof signature, 1, stdout);
+   put_chunk(IHDR, sizeof IHDR);
+
+   for(;;)
+      put_chunk(unknown, sizeof unknown);
+}

contrib/libtests/readpng.c

@@ -0,0 +1,104 @@
+/* readpng.c
+ *
+ * Copyright (c) 2013 John Cunningham Bowler
+ *
+ * Last changed in libpng 1.6.0 [(PENDING RELEASE)]
+ *
+ * This code is released under the libpng license.
+ * For conditions of distribution and use, see the disclaimer
+ * and license in png.h
+ *
+ * Load an arbitrary number of PNG files (from the command line, or, if there
+ * are no arguments on the command line, from stdin) then run a time test by
+ * reading each file by row.  The test does nothing with the read result and
+ * does no transforms.  The only output is a time as a floating point number of
+ * seconds with 9 decimal digits.
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#if (defined HAVE_CONFIG_H) && !(defined PNG_NO_CONFIG_H)
+#  include <config.h>
+#endif
+
+/* Define the following to use this test against your installed libpng, rather
+ * than the one being built here:
+ */
+#ifdef PNG_FREESTANDING_TESTS
+#  include <png.h>
+#else
+#  include "../../png.h"
+#endif
+
+static int
+read_png(FILE *fp)
+{
+   png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING,0,0,0);
+   png_infop info_ptr = NULL;
+   png_bytep row = NULL, display = NULL;
+
+   if (png_ptr == NULL)
+      return 0;
+
+   if (setjmp(png_jmpbuf(png_ptr)))
+   {
+      png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
+      if (row != NULL) free(row);
+      if (display != NULL) free(display);
+      return 0;
+   }
+
+   png_init_io(png_ptr, fp);
+
+   info_ptr = png_create_info_struct(png_ptr);
+   if (info_ptr == NULL)
+      png_error(png_ptr, "OOM allocating info structure");
+
+   png_set_keep_unknown_chunks(png_ptr, PNG_HANDLE_CHUNK_ALWAYS, NULL, 0);
+
+   png_read_info(png_ptr, info_ptr);
+
+   {
+      png_size_t rowbytes = png_get_rowbytes(png_ptr, info_ptr);
+
+      row = malloc(rowbytes);
+      display = malloc(rowbytes);
+
+      if (row == NULL || display == NULL)
+         png_error(png_ptr, "OOM allocating row buffers");
+
+      {
+         png_uint_32 height = png_get_image_height(png_ptr, info_ptr);
+         int passes = png_set_interlace_handling(png_ptr);
+         int pass;
+
+         png_start_read_image(png_ptr);
+
+         for (pass = 0; pass < passes; ++pass)
+         {
+            png_uint_32 y = height;
+
+            /* NOTE: this trashes the row each time; interlace handling won't
+             * work, but this avoids memory thrashing for speed testing.
+             */
+            while (y-- > 0)
+               png_read_row(png_ptr, row, display);
+         }
+      }
+   }
+
+   /* Make sure to read to the end of the file: */
+   png_read_end(png_ptr, info_ptr);
+   png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
+   free(row);
+   free(display);
+   return 1;
+}
+
+int
+main(void)
+{
+   /* Exit code 0 on success. */
+   return !read_png(stdin);
+}

pnginfo.h

@@ -223,7 +223,10 @@ defined(PNG_READ_BACKGROUND_SUPPORTED)
 #ifdef PNG_STORE_UNKNOWN_CHUNKS_SUPPORTED
    /* Storage for unknown chunks that the library doesn't recognize. */
    png_unknown_chunkp unknown_chunks;
-   unsigned int unknown_chunks_num;
+   /* The type of this field must match png_struct::user_chunk_cache_max,
+    * else overflow can occur.
+    */
+   png_uint_32 unknown_chunks_num;
 #endif
 
 #ifdef PNG_sPLT_SUPPORTED

pngset.c

@@ -1086,11 +1086,12 @@ check_location(png_const_structrp png_ptr, int location)
 
 void PNGAPI
 png_set_unknown_chunks(png_const_structrp png_ptr,
-   png_inforp info_ptr, png_const_unknown_chunkp unknowns, int num_unknowns)
+   png_inforp info_ptr, png_const_unknown_chunkp unknowns, int num_unknowns_in)
 {
+   png_uint_32 num_unknowns;
    png_unknown_chunkp np;
 
-   if (png_ptr == NULL || info_ptr == NULL || num_unknowns <= 0)
+   if (png_ptr == NULL || info_ptr == NULL || num_unknowns_in <= 0)
       return;
 
    /* Check for the failure cases where support has been disabled at compile
@@ -1121,9 +1122,24 @@ png_set_unknown_chunks(png_const_structrp png_ptr,
     * undefined behavior.  Changing to png_malloc fixes this by producing a
     * png_error.  The (png_size_t) cast was also removed as it hides a potential
     * overflow.
-    *
-    * TODO: fix the potential overflow in the multiply
     */
+   num_unknowns = (unsigned int)/*SAFE*/num_unknowns_in;
+
+   /* There are two overflow conditions, one on the count one on memory, on a
+    * 32-bit system the memory limit is critical, on a 64-bit system the count
+    * limit.
+    */
+   if (num_unknowns > PNG_UINT_32_MAX - info_ptr->unknown_chunks_num ||
+      num_unknowns > PNG_SIZE_MAX/(sizeof *np) - info_ptr->unknown_chunks_num)
+   {
+      /* This is a benign read error (user limits are disabled and we are about
+       * to overflow 2^32 chunks) and an application write error.
+       */
+      png_chunk_report(png_ptr, "too many unknown chunks",
+         PNG_CHUNK_WRITE_ERROR);
+      return;
+   }
+
    np = png_voidcast(png_unknown_chunkp, png_malloc(png_ptr,
        (info_ptr->unknown_chunks_num + (unsigned int)num_unknowns) *
        (sizeof (png_unknown_chunk))));
@@ -1140,8 +1156,8 @@ png_set_unknown_chunks(png_const_structrp png_ptr,
    /* Increment unknown_chunks_num each time round the loop to protect the
     * just-allocated chunk data.
     */
-   for (; --num_unknowns >= 0;
-      ++np, ++unknowns, ++(info_ptr->unknown_chunks_num))
+   for (; num_unknowns > 0;
+      --num_unknowns, ++np, ++unknowns, ++(info_ptr->unknown_chunks_num))
    {
       memcpy(np->name, unknowns->name, (sizeof unknowns->name));
       np->name[(sizeof np->name)-1] = '\0';