[devel] pngvalid: add memory overwrite and palette image checks

also minor cleanup in the libpng code itself (pngrtran.c and pngrutil.c) and some
extra checking there.

pngrtran.c

@@ -1106,6 +1106,10 @@ png_init_palette_transformations(png_structp png_ptr)
        (png_ptr->transformations & PNG_EXPAND))
    {
       {
+         /* TODO: THIS MUST BE WRONG, because in png_init_read_transformations
+          * below the background red,green,blue values are used directly in the
+          * palette case (allowing an out-of-palette background color!)
+          */
          png_ptr->background.red   =
              png_ptr->palette[png_ptr->background.index].red;
          png_ptr->background.green =
@@ -1375,7 +1379,7 @@ png_init_read_transformations(png_structp png_ptr)
     * PNG_BACKGROUND_IS_GRAY only to decide when to do the
     * png_do_gray_to_rgb() transformation.
     *
-    * NOTE: this code needs to be revised to avoid the complexity and
+    * TODO: this code needs to be revised to avoid the complexity and
     * interdependencies.  The color type of the background should be recorded in
     * png_set_background, along with the bit depth, then the code has a record
     * of exactly what color space the background is currently in.
@@ -1776,8 +1780,11 @@ png_read_transform_info(png_structp png_ptr, png_infop info_ptr)
    {
       if (info_ptr->color_type == PNG_COLOR_TYPE_PALETTE)
       {
-         if (png_ptr->num_trans &&
-             (png_ptr->transformations & PNG_EXPAND_tRNS))
+         /* This check must match what actually happens in
+          * png_do_expand_palette; if it every checks the tRNS chunk to see if
+          * it is all opaque we must do the same (at present it does not.)
+          */
+         if (png_ptr->num_trans > 0)
             info_ptr->color_type = PNG_COLOR_TYPE_RGB_ALPHA;
 
          else
@@ -1918,6 +1925,14 @@ defined(PNG_READ_USER_TRANSFORM_SUPPORTED)
 
    info_ptr->rowbytes = PNG_ROWBYTES(info_ptr->pixel_depth, info_ptr->width);
 
+   /* Adding in 1.5.3: cache the above value in png_struct so that we can later
+    * check in png_rowbytes that the user buffer won't get overwritten.  Note
+    * that the field is not always set - if png_read_update_info isn't called
+    * the application has to either not do any transforms or get the calculation
+    * right itself.
+    */
+   png_ptr->info_rowbytes = info_ptr->rowbytes;
+
 #ifndef PNG_READ_EXPAND_SUPPORTED
    if (png_ptr)
       return;

pngrutil.c

@@ -2670,6 +2670,14 @@ png_combine_row(png_structp png_ptr, png_bytep row, int mask)
 {
    png_debug(1, "in png_combine_row");
 
+   /* Added in 1.5.3: the row_info should match the information returned by any
+    * call to png_read_update_info at this point.  Do not continue if we got
+    * this wrong.
+    */
+   if (png_ptr->info_rowbytes != 0 && png_ptr->info_rowbytes !=
+          PNG_ROWBYTES(png_ptr->row_info.pixel_depth, png_ptr->width))
+      png_error(png_ptr, "internal row size calculation error");
+
    if (mask == 0xff)
    {
       png_memcpy(row, png_ptr->row_buf + 1,

pngstruct.h

@@ -111,6 +111,7 @@ struct png_struct_def
    png_bytep avg_row;         /* buffer to save "avg" row when filtering */
    png_bytep paeth_row;       /* buffer to save "Paeth" row when filtering */
    png_row_info row_info;     /* used for transformation routines */
+   png_size_t info_rowbytes;  /* Adding in 1.5.3: cache of updated row bytes */
 
    png_uint_32 idat_size;     /* current IDAT size for read */
    png_uint_32 crc;           /* current chunk CRC value */