[libpng15] Prevent libpng overwriting unused bits at the end of the image when
is not byte aligned, while reading. Prior to libpng-1.5.6 libpng would
overwrite the end of the image if the row width is not an exact multiple
of 8 bits and the image is not interlaced.
diff --git a/ANNOUNCE b/ANNOUNCE
index ce73b69..ca39fb4 100644
--- a/ANNOUNCE
+++ b/ANNOUNCE
@@ -80,6 +80,10 @@
twice results in inaccuracies that can't be easily fixed. There is now
a warning in the code if this is going to happen.
Turned on multiple png_read_update_info in pngvalid transform tests.
+ Prevent libpng overwriting unused bits at the end of the image when when
+ it is not byte aligned, while reading. Prior to libpng-1.5.6 libpng would
+ overwrite the end of the image if the row width is not an exact multiple
+ of 8 bits and the image is not interlaced.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net:
(subscription required; visit
diff --git a/CHANGES b/CHANGES
index 40b4550..a1be5fc 100644
--- a/CHANGES
+++ b/CHANGES
@@ -3641,6 +3641,10 @@
twice results in inaccuracies that can't be easily fixed. There is now
a warning in the code if this is going to happen.
Turned on multiple png_read_update_info in pngvalid transform tests.
+ Prevent libpng overwriting unused bits at the end of the image when when
+ it is not byte aligned, while reading. Prior to libpng-1.5.6 libpng would
+ overwrite the end of the image if the row width is not an exact multiple
+ of 8 bits and the image is not interlaced.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit
diff --git a/pngrutil.c b/pngrutil.c
index e5feb18..6a228a7 100644
--- a/pngrutil.c
+++ b/pngrutil.c
@@ -2786,6 +2786,9 @@
png_const_bytep sp = png_ptr->row_buf + 1;
png_uint_32 row_width = png_ptr->width;
unsigned int pass = png_ptr->pass;
+ png_bytep end_ptr = 0;
+ png_byte end_byte = 0;
+ unsigned int end_mask;
png_debug(1, "in png_combine_row");
@@ -2807,6 +2810,26 @@
if (row_width == 0)
png_error(png_ptr, "internal row width error");
+ /* Preserve the last byte in cases where only part of it will be overwritten,
+ * the multiply below may overflow, we don't care because ANSI-C guarantees
+ * we get the low bits.
+ */
+ end_mask = (pixel_depth * row_width) & 7;
+ if (end_mask != 0)
+ {
+ /* ep == NULL is a flag to say do nothing */
+ end_ptr = dp + PNG_ROWBYTES(pixel_depth, row_width) - 1;
+ end_byte = *end_ptr;
+# ifdef PNG_READ_PACKSWAP_SUPPORTED
+ if (png_ptr->transformations & PNG_PACKSWAP) /* little-endian byte */
+ end_mask = 0xff << end_mask;
+
+ else /* big-endian byte */
+# endif
+ end_mask = 0xff >> end_mask;
+ /* end_mask is now the bits to *keep* from the destination row */
+ }
+
/* This reduces to a memcpy for non-interlaced images and for the case where
* interlacing isn't supported or isn't done (in that case the caller gets a
* sequence of interlace pass rows.)
@@ -2969,7 +2992,7 @@
* this.
*/
if (row_width <= pixels_per_byte)
- return;
+ break; /* May need to restore part of the last byte */
row_width -= pixels_per_byte;
++dp;
@@ -3023,6 +3046,10 @@
/* And simply copy these bytes. Some optimization is possible here,
* depending on the value of 'bytes_to_copy'. Speical case the low
* byte counts, which we know to be frequent.
+ *
+ * Notice that these cases all 'return' rather than 'break' - this
+ * avoids an unnecessary test on whether to restore the last byte
+ * below.
*/
switch (bytes_to_copy)
{
@@ -3188,9 +3215,11 @@
bytes_to_copy = row_width;
}
}
+
+ /* NOT REACHED*/
} /* pixel_depth >= 8 */
- /* NOT REACHED*/
+ /* Here if pixel_depth < 8 to check 'end_ptr' below. */
}
else
#endif
@@ -3200,6 +3229,10 @@
* destination row if it is a partial byte.)
*/
png_memcpy(dp, sp, PNG_ROWBYTES(pixel_depth, row_width));
+
+ /* Restore the overwritten bits from the last byte if necessary. */
+ if (end_ptr != NULL)
+ *end_ptr = (png_byte)((end_byte & end_mask) | (*end_ptr & ~end_mask));
}
#ifdef PNG_READ_INTERLACING_SUPPORTED
diff --git a/pngvalid.c b/pngvalid.c
index eb19f52..434ce53 100644
--- a/pngvalid.c
+++ b/pngvalid.c
@@ -433,12 +433,35 @@
memmove(toBuffer+(toIndex>>3), fromBuffer+(fromIndex>>3), pixelSize>>3);
}
+/* Copy a complete row of pixels, taking into account potential partial
+ * bytes at the end.
+ */
+static void
+row_copy(png_bytep toBuffer, png_const_bytep fromBuffer, unsigned int bitWidth)
+{
+ memcpy(toBuffer, fromBuffer, bitWidth >> 3);
+
+ if ((bitWidth & 7) != 0)
+ {
+ unsigned int mask;
+
+ toBuffer += bitWidth >> 3;
+ fromBuffer += bitWidth >> 3;
+ /* The remaining bits are in the top of the byte, the mask is the bits to
+ * retain.
+ */
+ mask = 0xff >> (bitWidth & 7);
+ *toBuffer = (png_byte)((*toBuffer & mask) | (*fromBuffer & ~mask));
+ }
+}
+
/* Compare pixels - they are assumed to start at the first byte in the
* given buffers.
*/
static int
pixel_cmp(png_const_bytep pa, png_const_bytep pb, png_uint_32 bit_width)
{
+#if PNG_LIBPNG_VER < 10506
if (memcmp(pa, pb, bit_width>>3) == 0)
{
png_uint_32 p;
@@ -459,6 +482,13 @@
if (p == 0) return 0;
}
+#else
+ /* From libpng-1.5.6 the overwrite should be fixed, so compare the trailing
+ * bits too:
+ */
+ if (memcmp(pa, pb, (bit_width+7)>>3) == 0)
+ return 0;
+#endif
/* Return the index of the changed byte. */
{
@@ -908,11 +938,13 @@
ps->cb_row = cbRow;
ps->image_h = cRows;
- /* For error checking, the whole buffer is set to '1' - this matches what
- * happens with the 'size' test images on write and also matches the unused
- * bits in the test rows.
+ /* For error checking, the whole buffer is set to 10110010 (0xb2 - 178).
+ * This deliberately doesn't match the bits in the size test image which are
+ * outside the image; these are set to 0xff (all 1). To make the row
+ * comparison work in the 'size' test case the size rows are pre-initialized
+ * to the same value prior to calling 'standard_row'.
*/
- memset(ps->image, 0xff, cb);
+ memset(ps->image, 178, cb);
/* Then put in the marks. */
while (--nImages >= 0)
@@ -4360,7 +4392,7 @@
if (dp->interlace_type == PNG_INTERLACE_ADAM7)
deinterlace_row(row, new_row, dp->pixel_size, dp->w, pass);
else
- memcpy(row, new_row, dp->cbRow);
+ row_copy(row, new_row, dp->pixel_size * dp->w);
}
else
png_progressive_combine_row(pp, row, new_row);
@@ -4404,10 +4436,12 @@
/* The following aids (to some extent) error detection - we can
* see where png_read_row wrote. Use opposite values in row and
- * display to make this easier.
+ * display to make this easier. Don't use 0xff (which is used in
+ * the image write code to fill unused bits) or 0 (which is a
+ * likely value to overwrite unused bits with).
*/
- memset(row, 0xff, sizeof row);
- memset(display, 0, sizeof display);
+ memset(row, 0xc5, sizeof row);
+ memset(display, 0x5c, sizeof display);
png_read_row(pp, row, display);
@@ -4440,39 +4474,51 @@
int where;
png_byte std[STANDARD_ROWMAX];
- memset(std, 0xff, sizeof std);
+ /* The row must be pre-initialized to the magic number here for the size
+ * tests to pass:
+ */
+ memset(std, 178, sizeof std);
standard_row(pp, std, dp->id, y);
/* At the end both the 'row' and 'display' arrays should end up identical.
* In earlier passes 'row' will be partially filled in, with only the pixels
* that have been read so far, but 'display' will have those pixels
* replicated to fill the unread pixels while reading an interlaced image.
+#if PNG_LIBPNG_VER < 10506
* The side effect inside the libpng sequential reader is that the 'row'
* array retains the correct values for unwritten pixels within the row
* bytes, while the 'display' array gets bits off the end of the image (in
* the last byte) trashed. Unfortunately in the progressive reader the
* row bytes are always trashed, so we always do a pixel_cmp here even though
* a memcmp of all cbRow bytes will succeed for the sequential reader.
+#endif
*/
if (iImage >= 0 &&
(where = pixel_cmp(std, store_image_row(dp->ps, pp, iImage, y),
dp->bit_width)) != 0)
{
char msg[64];
- sprintf(msg, "PNG image row %d changed at byte %d", y, where-1);
+ sprintf(msg, "PNG image row[%d][%d] changed from %.2x to %.2x", y,
+ where-1, std[where-1],
+ store_image_row(dp->ps, pp, iImage, y)[where-1]);
png_error(pp, msg);
}
+#if PNG_LIBPNG_VER < 10506
/* In this case use pixel_cmp because we need to compare a partial
* byte at the end of the row if the row is not an exact multiple
- * of 8 bits wide.
+ * of 8 bits wide. (This is fixed in libpng-1.5.6 and pixel_cmp is
+ * changed to match!)
*/
+#endif
if (iDisplay >= 0 &&
(where = pixel_cmp(std, store_image_row(dp->ps, pp, iDisplay, y),
dp->bit_width)) != 0)
{
char msg[64];
- sprintf(msg, "display row %d changed at byte %d", y, where-1);
+ sprintf(msg, "display row[%d][%d] changed from %.2x to %.2x", y,
+ where-1, std[where-1],
+ store_image_row(dp->ps, pp, iDisplay, y)[where-1]);
png_error(pp, msg);
}
}
