Hello, The following remarks about my previous patch came up on the Debian QA mailing list.
On Thu, 26 Jan 2006, Russ Allbery wrote: > Ugh, that's not a very good solution. That means that the next time > there's a security hole in zlib or libpng, which hasn't been uncommon, it > may also affect pngcrush and someone has to remember to update it as well. > This makes life much harder for the security team. And On Fri, 27 Jan 2006, Bas Zoetekouw wrote: > Please don't do that, it will make everyone's live (en specifically > those of the security team) much harder. I have prepared a new patch for pngcrush. The diff is attached. What it does is to add a source file (pngpf_for_crush.c) that contains some "innocent looking" functions from libpng which were declared "PRIVATE" in the recent version (1.2.8rel) of libpng. I agree that this is still a far from ideal solution but: a. The functions indeed appear to be quite innocent. In particular, these functions are built out of non PRIVATE functions and modify data structures that are non PRIVATE. b. The functions are copied verbatim from the latest libpng source but have essentially not changed from 2002 versions of the same functions. c. The size of this added file is less than 10K so it should be easy to check it for bugs or security holes in principle. d. With these changes, pngcrush builds (and works) based on the existing libpng-dev and zlib1g-dev packages. Thanks and regards, Kapil. --
diff -u pngcrush-1.5.10/Makefile pngcrush-1.5.10/Makefile
--- pngcrush-1.5.10/Makefile
+++ pngcrush-1.5.10/Makefile
@@ -5,8 +5,8 @@
all: pngcrush
-pngcrush: pngcrush.o
- $(CC) -o pngcrush -lpng -lz pngcrush.o
+pngcrush: pngcrush.o pngpf_for_crush.o
+ $(CC) -o pngcrush -lpng -lz pngpf_for_crush.o pngcrush.o
install:
--- pngcrush-1.5.10.orig/pngpf_for_crush.c
+++ pngcrush-1.5.10/pngpf_for_crush.c
@@ -0,0 +1,342 @@
+/* pngpf_for_crush.c - some private functions from libpng
+ * incorporated for use with pngcrush
+ *
+ * This code is taken from the listed files in
+ * libpng all of which have the following copyright statement.
+ *
+ * Note that pngcrush is also written by Glenn Randers-Pehrson.
+ *
+ * libpng version 1.2.8 - December 3, 2004
+ * For conditions of distribution and use, see copyright notice in png.h
+ * Copyright (c) 1998-2004 Glenn Randers-Pehrson
+ * (Version 0.96 Copyright (c) 1996, 1997 Andreas Dilger)
+ * (Version 0.88 Copyright (c) 1995, 1996 Guy Eric Schalnat, Group 42, Inc.)
+ *
+*/
+
+#define PNG_INTERNAL
+#include <png.h>
+
+/* Code taken from:
+ *
+ * png.c
+ *
+ */
+
+/* Reset the CRC variable to 32 bits of 1's. Care must be taken
+ * in case CRC is > 32 bits to leave the top bits 0.
+ */
+void /* PRIVATE */
+png_reset_crc(png_structp png_ptr)
+{
+ png_ptr->crc = crc32(0, Z_NULL, 0);
+}
+
+/* Calculate the CRC over a section of data. We can only pass as
+ * much data to this routine as the largest single buffer size. We
+ * also check that this data will actually be used before going to the
+ * trouble of calculating it.
+ */
+void /* PRIVATE */
+png_calculate_crc(png_structp png_ptr, png_bytep ptr, png_size_t length)
+{
+ int need_crc = 1;
+
+ if (png_ptr->chunk_name[0] & 0x20) /* ancillary */
+ {
+ if ((png_ptr->flags & PNG_FLAG_CRC_ANCILLARY_MASK) ==
+ (PNG_FLAG_CRC_ANCILLARY_USE | PNG_FLAG_CRC_ANCILLARY_NOWARN))
+ need_crc = 0;
+ }
+ else /* critical */
+ {
+ if (png_ptr->flags & PNG_FLAG_CRC_CRITICAL_IGNORE)
+ need_crc = 0;
+ }
+
+ if (need_crc)
+ png_ptr->crc = crc32(png_ptr->crc, ptr, (uInt)length);
+}
+
+/* Code taken from:
+ *
+ * pngrio.c
+ *
+ */
+
+/* Read the data from whatever input you are using. The default routine
+ reads from a file pointer. Note that this routine sometimes gets called
+ with very small lengths, so you should implement some kind of simple
+ buffering if you are using unbuffered reads. This should never be asked
+ to read more then 64K on a 16 bit machine. */
+void /* PRIVATE */
+png_read_data(png_structp png_ptr, png_bytep data, png_size_t length)
+{
+ png_debug1(4,"reading %d bytes\n", (int)length);
+ if (png_ptr->read_data_fn != NULL)
+ (*(png_ptr->read_data_fn))(png_ptr, data, length);
+ else
+ png_error(png_ptr, "Call to NULL read function");
+}
+
+/* Code taken from:
+ *
+ * pngrtran.c
+ *
+ */
+
+/* Modify the info structure to reflect the transformations. The
+ * info should be updated so a PNG file could be written with it,
+ * assuming the transformations result in valid PNG data.
+ */
+void /* PRIVATE */
+png_read_transform_info(png_structp png_ptr, png_infop info_ptr)
+{
+ png_debug(1, "in png_read_transform_info\n");
+#if defined(PNG_READ_EXPAND_SUPPORTED)
+ if (png_ptr->transformations & PNG_EXPAND)
+ {
+ if (info_ptr->color_type == PNG_COLOR_TYPE_PALETTE)
+ {
+ if (png_ptr->num_trans)
+ info_ptr->color_type = PNG_COLOR_TYPE_RGB_ALPHA;
+ else
+ info_ptr->color_type = PNG_COLOR_TYPE_RGB;
+ info_ptr->bit_depth = 8;
+ info_ptr->num_trans = 0;
+ }
+ else
+ {
+ if (png_ptr->num_trans)
+ info_ptr->color_type |= PNG_COLOR_MASK_ALPHA;
+ if (info_ptr->bit_depth < 8)
+ info_ptr->bit_depth = 8;
+ info_ptr->num_trans = 0;
+ }
+ }
+#endif
+
+#if defined(PNG_READ_BACKGROUND_SUPPORTED)
+ if (png_ptr->transformations & PNG_BACKGROUND)
+ {
+ info_ptr->color_type &= ~PNG_COLOR_MASK_ALPHA;
+ info_ptr->num_trans = 0;
+ info_ptr->background = png_ptr->background;
+ }
+#endif
+
+#if defined(PNG_READ_GAMMA_SUPPORTED)
+ if (png_ptr->transformations & PNG_GAMMA)
+ {
+#ifdef PNG_FLOATING_POINT_SUPPORTED
+ info_ptr->gamma = png_ptr->gamma;
+#endif
+#ifdef PNG_FIXED_POINT_SUPPORTED
+ info_ptr->int_gamma = png_ptr->int_gamma;
+#endif
+ }
+#endif
+
+#if defined(PNG_READ_16_TO_8_SUPPORTED)
+ if ((png_ptr->transformations & PNG_16_TO_8) && (info_ptr->bit_depth == 16))
+ info_ptr->bit_depth = 8;
+#endif
+
+#if defined(PNG_READ_DITHER_SUPPORTED)
+ if (png_ptr->transformations & PNG_DITHER)
+ {
+ if (((info_ptr->color_type == PNG_COLOR_TYPE_RGB) ||
+ (info_ptr->color_type == PNG_COLOR_TYPE_RGB_ALPHA)) &&
+ png_ptr->palette_lookup && info_ptr->bit_depth == 8)
+ {
+ info_ptr->color_type = PNG_COLOR_TYPE_PALETTE;
+ }
+ }
+#endif
+
+#if defined(PNG_READ_PACK_SUPPORTED)
+ if ((png_ptr->transformations & PNG_PACK) && (info_ptr->bit_depth < 8))
+ info_ptr->bit_depth = 8;
+#endif
+
+#if defined(PNG_READ_GRAY_TO_RGB_SUPPORTED)
+ if (png_ptr->transformations & PNG_GRAY_TO_RGB)
+ info_ptr->color_type |= PNG_COLOR_MASK_COLOR;
+#endif
+
+#if defined(PNG_READ_RGB_TO_GRAY_SUPPORTED)
+ if (png_ptr->transformations & PNG_RGB_TO_GRAY)
+ info_ptr->color_type &= ~PNG_COLOR_MASK_COLOR;
+#endif
+
+ if (info_ptr->color_type == PNG_COLOR_TYPE_PALETTE)
+ info_ptr->channels = 1;
+ else if (info_ptr->color_type & PNG_COLOR_MASK_COLOR)
+ info_ptr->channels = 3;
+ else
+ info_ptr->channels = 1;
+
+#if defined(PNG_READ_STRIP_ALPHA_SUPPORTED)
+ if (png_ptr->flags & PNG_FLAG_STRIP_ALPHA)
+ info_ptr->color_type &= ~PNG_COLOR_MASK_ALPHA;
+#endif
+
+ if (info_ptr->color_type & PNG_COLOR_MASK_ALPHA)
+ info_ptr->channels++;
+
+#if defined(PNG_READ_FILLER_SUPPORTED)
+ /* STRIP_ALPHA and FILLER allowed: MASK_ALPHA bit stripped above */
+ if ((png_ptr->transformations & PNG_FILLER) &&
+ ((info_ptr->color_type == PNG_COLOR_TYPE_RGB) ||
+ (info_ptr->color_type == PNG_COLOR_TYPE_GRAY)))
+ {
+ info_ptr->channels++;
+ /* if adding a true alpha channel not just filler */
+#if !defined(PNG_1_0_X)
+ if (png_ptr->transformations & PNG_ADD_ALPHA)
+ info_ptr->color_type |= PNG_COLOR_MASK_ALPHA;
+#endif
+ }
+#endif
+
+#if defined(PNG_USER_TRANSFORM_PTR_SUPPORTED) && \
+defined(PNG_READ_USER_TRANSFORM_SUPPORTED)
+ if(png_ptr->transformations & PNG_USER_TRANSFORM)
+ {
+ if(info_ptr->bit_depth < png_ptr->user_transform_depth)
+ info_ptr->bit_depth = png_ptr->user_transform_depth;
+ if(info_ptr->channels < png_ptr->user_transform_channels)
+ info_ptr->channels = png_ptr->user_transform_channels;
+ }
+#endif
+
+ info_ptr->pixel_depth = (png_byte)(info_ptr->channels *
+ info_ptr->bit_depth);
+
+ info_ptr->rowbytes = PNG_ROWBYTES(info_ptr->pixel_depth,info_ptr->width);
+
+#if !defined(PNG_READ_EXPAND_SUPPORTED)
+ if(png_ptr)
+ return;
+#endif
+}
+
+/* Code taken from:
+ *
+ * pngrutil.c
+ *
+ */
+
+#ifndef PNG_READ_BIG_ENDIAN_SUPPORTED
+/* Grab an unsigned 32-bit integer from a buffer in big-endian format. */
+png_uint_32 /* PRIVATE */
+png_get_uint_32(png_bytep buf)
+{
+ png_uint_32 i = ((png_uint_32)(*buf) << 24) +
+ ((png_uint_32)(*(buf + 1)) << 16) +
+ ((png_uint_32)(*(buf + 2)) << 8) +
+ (png_uint_32)(*(buf + 3));
+
+ return (i);
+}
+#endif /* PNG_READ_BIG_ENDIAN_SUPPORTED */
+
+/* Read data, and (optionally) run it through the CRC. */
+void /* PRIVATE */
+png_crc_read(png_structp png_ptr, png_bytep buf, png_size_t length)
+{
+ png_read_data(png_ptr, buf, length);
+ png_calculate_crc(png_ptr, buf, length);
+}
+
+/* Optionally skip data and then check the CRC. Depending on whether we
+ are reading a ancillary or critical chunk, and how the program has set
+ things up, we may calculate the CRC on the data and print a message.
+ Returns '1' if there was a CRC error, '0' otherwise. */
+int /* PRIVATE */
+png_crc_finish(png_structp png_ptr, png_uint_32 skip)
+{
+ png_size_t i;
+ png_size_t istop = png_ptr->zbuf_size;
+
+ for (i = (png_size_t)skip; i > istop; i -= istop)
+ {
+ png_crc_read(png_ptr, png_ptr->zbuf, png_ptr->zbuf_size);
+ }
+ if (i)
+ {
+ png_crc_read(png_ptr, png_ptr->zbuf, i);
+ }
+
+ if (png_crc_error(png_ptr))
+ {
+ if (((png_ptr->chunk_name[0] & 0x20) && /* Ancillary */
+ !(png_ptr->flags & PNG_FLAG_CRC_ANCILLARY_NOWARN)) ||
+ (!(png_ptr->chunk_name[0] & 0x20) && /* Critical */
+ (png_ptr->flags & PNG_FLAG_CRC_CRITICAL_USE)))
+ {
+ png_chunk_warning(png_ptr, "CRC error");
+ }
+ else
+ {
+ png_chunk_error(png_ptr, "CRC error");
+ }
+ return (1);
+ }
+
+ return (0);
+}
+
+/* Compare the CRC stored in the PNG file with that calculated by libpng from
+ the data it has read thus far. */
+int /* PRIVATE */
+png_crc_error(png_structp png_ptr)
+{
+ png_byte crc_bytes[4];
+ png_uint_32 crc;
+ int need_crc = 1;
+
+ if (png_ptr->chunk_name[0] & 0x20) /* ancillary */
+ {
+ if ((png_ptr->flags & PNG_FLAG_CRC_ANCILLARY_MASK) ==
+ (PNG_FLAG_CRC_ANCILLARY_USE | PNG_FLAG_CRC_ANCILLARY_NOWARN))
+ need_crc = 0;
+ }
+ else /* critical */
+ {
+ if (png_ptr->flags & PNG_FLAG_CRC_CRITICAL_IGNORE)
+ need_crc = 0;
+ }
+
+ png_read_data(png_ptr, crc_bytes, 4);
+
+ if (need_crc)
+ {
+ crc = png_get_uint_32(crc_bytes);
+ return ((int)(crc != png_ptr->crc));
+ }
+ else
+ return (0);
+}
+
+/* Code taken from:
+ *
+ * pngwio.c
+ *
+ */
+
+/* Write the data to whatever output you are using. The default routine
+ writes to a file pointer. Note that this routine sometimes gets called
+ with very small lengths, so you should implement some kind of simple
+ buffering if you are using unbuffered writes. This should never be asked
+ to write more than 64K on a 16 bit machine. */
+
+void /* PRIVATE */
+png_write_data(png_structp png_ptr, png_bytep data, png_size_t length)
+{
+ if (png_ptr->write_data_fn != NULL )
+ (*(png_ptr->write_data_fn))(png_ptr, data, length);
+ else
+ png_error(png_ptr, "Call to NULL write function");
+}
+
signature.asc
Description: Digital signature
