refactor: Clean up the checking of chunk lengths and allocation limits
Internal changes only.
Move chunk length checks to fewer places:
Change `png_struct::user_chunk_malloc_max` to always have a non-zero
value, in order to avoid the need to check for zero in multiple places.
Add `png_chunk_max(png_ptr)`, a function-like macro defined in pngpriv.h
which expresses all the previous checks on the various USER_LIMITS and
system limitations. Replace the code which implemented such checks with
`png_chunk_max`.
Move the malloc limit length check in `png_read_chunk_header` to
`png_handle_chunk` and make it conditional on the chunk type.
Progressive reader: call `png_read_chunk_header`.
Correct the handling of pHYs.
Reviewed-by: Cosmin Truta <ctruta@gmail.com>
Signed-off-by: John Bowler <jbowler@acm.org>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>
diff --git a/png.c b/png.c
index 0ce0531..830b642 100644
--- a/png.c
+++ b/png.c
@@ -278,11 +278,18 @@
create_struct.user_chunk_cache_max = PNG_USER_CHUNK_CACHE_MAX;
# endif
-# ifdef PNG_USER_CHUNK_MALLOC_MAX
/* Added at libpng-1.2.43 and 1.4.1, required only for read but exists
* in png_struct regardless.
*/
+# if PNG_USER_CHUNK_MALLOC_MAX > 0 /* default to compile-time limit */
create_struct.user_chunk_malloc_max = PNG_USER_CHUNK_MALLOC_MAX;
+
+ /* No compile time limit so initialize to the system limit: */
+# elif (defined PNG_MAX_MALLOC_64K/* legacy system limit */
+ create_struct.user_chunk_malloc_max = 65536U;
+
+# else /* modern system limit SIZE_MAX (C99) */
+ create_struct.user_chunk_malloc_max = PNG_SIZE_MAX;
# endif
# endif
diff --git a/pngmem.c b/pngmem.c
index d391c13..90c13b1 100644
--- a/pngmem.c
+++ b/pngmem.c
@@ -72,30 +72,29 @@
* to implement a user memory handler. This checks to be sure it isn't
* called with big numbers.
*/
-#ifndef PNG_USER_MEM_SUPPORTED
- PNG_UNUSED(png_ptr)
-#endif
+# ifdef PNG_MAX_MALLOC_64K
+ /* This is support for legacy systems which had segmented addressing
+ * limiting the maximum allocation size to 65536. It takes precedence
+ * over PNG_SIZE_MAX which is set to 65535 on true 16-bit systems.
+ *
+ * TODO: libpng-1.8: finally remove both cases.
+ */
+ if (size > 65536U) return NULL;
+# endif
- /* Some compilers complain that this is always true. However, it
- * can be false when integer overflow happens.
+ /* This is checked too because the system malloc call below takes a (size_t).
*/
- if (size > 0 && size <= PNG_SIZE_MAX
-# ifdef PNG_MAX_MALLOC_64K
- && size <= 65536U
-# endif
- )
- {
-#ifdef PNG_USER_MEM_SUPPORTED
+ if (size > PNG_SIZE_MAX) return NULL;
+
+# ifdef PNG_USER_MEM_SUPPORTED
if (png_ptr != NULL && png_ptr->malloc_fn != NULL)
return png_ptr->malloc_fn(png_constcast(png_structrp,png_ptr), size);
+# else
+ PNG_UNUSED(png_ptr)
+# endif
- else
-#endif
- return malloc((size_t)size); /* checked for truncation above */
- }
-
- else
- return NULL;
+ /* Use the system malloc */
+ return malloc((size_t)/*SAFE*/size); /* checked for truncation above */
}
#if defined(PNG_TEXT_SUPPORTED) || defined(PNG_sPLT_SUPPORTED) ||\
diff --git a/pngpread.c b/pngpread.c
index 121c9db..60d8106 100644
--- a/pngpread.c
+++ b/pngpread.c
@@ -193,17 +193,8 @@
*/
if ((png_ptr->mode & PNG_HAVE_CHUNK_HEADER) == 0)
{
- png_byte chunk_length[4];
- png_byte chunk_tag[4];
-
PNG_PUSH_SAVE_BUFFER_IF_LT(8)
- png_push_fill_buffer(png_ptr, chunk_length, 4);
- png_ptr->push_length = png_get_uint_31(png_ptr, chunk_length);
- png_reset_crc(png_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->push_length);
+ png_ptr->push_length = png_read_chunk_header(png_ptr);
png_ptr->mode |= PNG_HAVE_CHUNK_HEADER;
}
diff --git a/pngpriv.h b/pngpriv.h
index 8f190ae..0be3efc 100644
--- a/pngpriv.h
+++ b/pngpriv.h
@@ -1081,6 +1081,25 @@
PNG_INTERNAL_FUNCTION(int,png_user_version_check,(png_structrp png_ptr,
png_const_charp user_png_ver),PNG_EMPTY);
+#ifdef PNG_READ_SUPPORTED /* should only be used on read */
+/* Security: read limits on the largest allocations while reading a PNG. This
+ * avoids very large allocations caused by PNG files with damaged or altered
+ * chunk 'length' fields.
+ */
+#ifdef PNG_SET_USER_LIMITS_SUPPORTED /* run-time limit */
+# define png_chunk_max(png_ptr) ((png_ptr)->user_chunk_malloc_max)
+
+#elif PNG_USER_CHUNK_MALLOC_MAX > 0 /* compile-time limit */
+# define png_chunk_max(png_ptr) ((void)png_ptr, PNG_USER_CHUNK_MALLOC_MAX)
+
+#elif (defined PNG_MAX_MALLOC_64K) /* legacy system limit */
+# define png_chunk_max(png_ptr) ((void)png_ptr, 65536U)
+
+#else /* modern system limit SIZE_MAX (C99) */
+# define png_chunk_max(png_ptr) ((void)png_ptr, PNG_SIZE_MAX)
+#endif
+#endif /* READ */
+
/* Internal base allocator - no messages, NULL on failure to allocate. This
* does, however, call the application provided allocator and that could call
* png_error (although that would be a bug in the application implementation.)
@@ -1584,12 +1603,6 @@
handled_ok /* known, supported and handled without error */
} png_handle_result_code;
-PNG_INTERNAL_FUNCTION(void,png_check_chunk_name,(png_const_structrp png_ptr,
- png_uint_32 chunk_name),PNG_EMPTY);
-
-PNG_INTERNAL_FUNCTION(void,png_check_chunk_length,(png_const_structrp png_ptr,
- png_uint_32 chunk_length),PNG_EMPTY);
-
PNG_INTERNAL_FUNCTION(png_handle_result_code,png_handle_unknown,
(png_structrp png_ptr, png_inforp info_ptr, png_uint_32 length, int keep),
PNG_EMPTY);
diff --git a/pngrutil.c b/pngrutil.c
index f7aa12a..e1354b7 100644
--- a/pngrutil.c
+++ b/pngrutil.c
@@ -144,6 +144,38 @@
png_ptr->mode |= PNG_HAVE_PNG_SIGNATURE;
}
+/* This function is called to verify that a chunk name is valid.
+ * Do this using the bit-whacking approach from contrib/tools/pngfix.c
+ *
+ * Copied from libpng 1.7.
+ */
+static int
+check_chunk_name(png_uint_32 name)
+{
+ png_uint_32 t;
+
+ /* Remove bit 5 from all but the reserved byte; this means
+ * every 8-bit unit must be in the range 65-90 to be valid.
+ * So bit 5 must be zero, bit 6 must be set and bit 7 zero.
+ */
+ name &= ~PNG_U32(32,32,0,32);
+ t = (name & ~0x1f1f1f1fU) ^ 0x40404040U;
+
+ /* Subtract 65 for each 8-bit quantity, this must not
+ * overflow and each byte must then be in the range 0-25.
+ */
+ name -= PNG_U32(65,65,65,65);
+ t |= name;
+
+ /* Subtract 26, handling the overflow which should set the
+ * top three bits of each byte.
+ */
+ name -= PNG_U32(25,25,25,26);
+ t |= ~name;
+
+ return (t & 0xe0e0e0e0U) == 0U;
+}
+
/* Read the chunk header (length + type name).
* Put the type name into png_ptr->chunk_name, and return the length.
*/
@@ -151,33 +183,36 @@
png_read_chunk_header(png_structrp png_ptr)
{
png_byte buf[8];
- png_uint_32 length;
+ png_uint_32 chunk_name, length;
#ifdef PNG_IO_STATE_SUPPORTED
png_ptr->io_state = PNG_IO_READING | PNG_IO_CHUNK_HDR;
#endif
- /* Read the length and the chunk name.
- * This must be performed in a single I/O call.
+ /* Read the length and the chunk name. png_struct::chunk_name is immediately
+ * updated even if they are detectably wrong. This aids error message
+ * handling by allowing png_chunk_error to be used.
*/
png_read_data(png_ptr, buf, 8);
length = png_get_uint_31(png_ptr, buf);
-
- /* Put the chunk name into png_ptr->chunk_name. */
- png_ptr->chunk_name = PNG_CHUNK_FROM_STRING(buf+4);
-
- png_debug2(0, "Reading chunk typeid = 0x%lx, length = %lu",
- (unsigned long)png_ptr->chunk_name, (unsigned long)length);
+ png_ptr->chunk_name = chunk_name = PNG_CHUNK_FROM_STRING(buf+4);
/* Reset the crc and run it over the chunk name. */
png_reset_crc(png_ptr);
png_calculate_crc(png_ptr, buf + 4, 4);
- /* Check to see if chunk name is valid. */
- png_check_chunk_name(png_ptr, png_ptr->chunk_name);
+ png_debug2(0, "Reading chunk typeid = 0x%lx, length = %lu",
+ (unsigned long)png_ptr->chunk_name, (unsigned long)length);
- /* Check for too-large chunk length */
- png_check_chunk_length(png_ptr, length);
+ /* Sanity check the length (first by <= 0x80) and the chunk name. An error
+ * here indicates a broken stream and libpng has no recovery from this.
+ */
+ if (buf[0] >= 0x80U)
+ png_chunk_error(png_ptr, "bad header (invalid length)");
+
+ /* Check to see if chunk name is valid. */
+ if (!check_chunk_name(chunk_name))
+ png_chunk_error(png_ptr, "bad header (invalid type)");
#ifdef PNG_IO_STATE_SUPPORTED
png_ptr->io_state = PNG_IO_READING | PNG_IO_CHUNK_DATA;
@@ -335,17 +370,14 @@
/* Manage the read buffer; this simply reallocates the buffer if it is not small
* enough (or if it is not allocated). The routine returns a pointer to the
* buffer; if an error occurs and 'warn' is set the routine returns NULL, else
- * it will call png_error (via png_malloc) on failure. (warn == 2 means
- * 'silent').
+ * it will call png_error on failure.
*/
static png_bytep
-png_read_buffer(png_structrp png_ptr, png_alloc_size_t new_size, int warn)
+png_read_buffer(png_structrp png_ptr, png_alloc_size_t new_size)
{
png_bytep buffer = png_ptr->read_buffer;
- /* TODO: put the code for checking the CHUNK_MALLOC_MAX in here to avoid
- * duplication and to avoid unexpected errors.
- */
+ if (new_size > png_chunk_max(png_ptr)) return NULL;
if (buffer != NULL && new_size > png_ptr->read_buffer_size)
{
@@ -361,19 +393,12 @@
if (buffer != NULL)
{
- memset(buffer, 0, new_size); /* just in case */
+# ifndef PNG_NO_MEMZERO /* for detecting UIM bugs **only** */
+ memset(buffer, 0, new_size); /* just in case */
+# endif
png_ptr->read_buffer = buffer;
png_ptr->read_buffer_size = new_size;
}
-
- else if (warn < 2) /* else silent */
- {
- if (warn != 0)
- png_chunk_warning(png_ptr, "insufficient memory to read chunk");
-
- else
- png_chunk_error(png_ptr, "insufficient memory to read chunk");
- }
}
return buffer;
@@ -665,16 +690,7 @@
* maybe a '\0' terminator too. We have to assume that 'prefix_size' is
* limited only by the maximum chunk size.
*/
- png_alloc_size_t limit = PNG_SIZE_MAX;
-
-# 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
+ png_alloc_size_t limit = png_chunk_max(png_ptr);
if (limit >= prefix_size + (terminate != 0))
{
@@ -879,87 +895,6 @@
}
#endif /* READ_iCCP */
-/* This function is called to verify that a chunk name is valid.
- * Do this using the bit-whacking approach from contrib/tools/pngfix.c
- *
- * Copied from libpng 1.7.
- */
-static int
-check_chunk_name(png_uint_32 name)
-{
- png_uint_32 t;
-
- /* Remove bit 5 from all but the reserved byte; this means
- * every 8-bit unit must be in the range 65-90 to be valid.
- * So bit 5 must be zero, bit 6 must be set and bit 7 zero.
- */
- name &= ~PNG_U32(32,32,0,32);
- t = (name & ~0x1f1f1f1fU) ^ 0x40404040U;
-
- /* Subtract 65 for each 8-bit quantity, this must not
- * overflow and each byte must then be in the range 0-25.
- */
- name -= PNG_U32(65,65,65,65);
- t |= name;
-
- /* Subtract 26, handling the overflow which should set the
- * top three bits of each byte.
- */
- name -= PNG_U32(25,25,25,26);
- t |= ~name;
-
- return (t & 0xe0e0e0e0U) == 0U;
-}
-
-void /* PRIVATE */
-png_check_chunk_name(png_const_structrp png_ptr, png_uint_32 chunk_name)
-{
- if (check_chunk_name(chunk_name))
- return;
-
- png_chunk_error(png_ptr, "invalid chunk type");
-}
-
-void /* PRIVATE */
-png_check_chunk_length(png_const_structrp png_ptr, png_uint_32 length)
-{
- png_alloc_size_t limit = PNG_UINT_31_MAX;
-
-# 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->chunk_name == png_IDAT)
- {
- png_alloc_size_t idat_limit = PNG_UINT_31_MAX;
- size_t row_factor =
- (size_t)png_ptr->width
- * (size_t)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)
- idat_limit = PNG_UINT_31_MAX;
- else
- idat_limit = png_ptr->height * row_factor;
- row_factor = row_factor > 32566? 32566 : row_factor;
- idat_limit += 6 + 5*(idat_limit/row_factor+1); /* zlib+deflate overhead */
- idat_limit=idat_limit < PNG_UINT_31_MAX? idat_limit : PNG_UINT_31_MAX;
- limit = limit < idat_limit? idat_limit : limit;
- }
-
- if (length > limit)
- {
- png_debug2(0," length = %lu, limit = %lu",
- (unsigned long)length,(unsigned long)limit);
- png_benign_error(png_ptr, "chunk data is too large");
- }
-}
-
/* CHUNK HANDLING */
/* Read and check the IDHR chunk */
static png_handle_result_code
@@ -1486,7 +1421,7 @@
png_uint_32 tag_count =
png_get_uint_32(profile_header + 128);
png_bytep profile = png_read_buffer(png_ptr,
- profile_length, 2/*silent*/);
+ profile_length);
if (profile != NULL)
{
@@ -1658,16 +1593,7 @@
}
#endif
-#ifdef PNG_MAX_MALLOC_64K
- if (length > 65535U)
- {
- png_crc_finish(png_ptr, length);
- png_chunk_benign_error(png_ptr, "too large to fit in memory");
- return handled_error;
- }
-#endif
-
- buffer = png_read_buffer(png_ptr, length+1, 2/*silent*/);
+ buffer = png_read_buffer(png_ptr, length+1);
if (buffer == NULL)
{
png_crc_finish(png_ptr, length);
@@ -2088,12 +2014,11 @@
static png_handle_result_code /* PRIVATE */
png_handle_eXIf(png_structrp png_ptr, png_inforp info_ptr, png_uint_32 length)
{
- /* TODO: rewrite to use png_read_buffer, like everything else. */
png_bytep buffer = NULL;
png_debug(1, "in png_handle_eXIf");
- buffer = png_read_buffer(png_ptr, length, 2/*silent*/);
+ buffer = png_read_buffer(png_ptr, length);
if (buffer == NULL)
{
@@ -2240,7 +2165,7 @@
png_debug1(2, "Allocating and reading pCAL chunk data (%u bytes)",
length + 1);
- buffer = png_read_buffer(png_ptr, length+1, 2/*silent*/);
+ buffer = png_read_buffer(png_ptr, length+1);
if (buffer == NULL)
{
@@ -2358,12 +2283,12 @@
png_debug1(2, "Allocating and reading sCAL chunk data (%u bytes)",
length + 1);
- buffer = png_read_buffer(png_ptr, length+1, 2/*silent*/);
+ buffer = png_read_buffer(png_ptr, length+1);
if (buffer == NULL)
{
- png_chunk_benign_error(png_ptr, "out of memory");
png_crc_finish(png_ptr, length);
+ png_chunk_benign_error(png_ptr, "out of memory");
return handled_error;
}
@@ -2487,24 +2412,15 @@
}
#endif
- /* TODO: this doesn't work and shouldn't be necessary (see above). */
+ /* TODO: this doesn't work and shouldn't be necessary. */
if ((png_ptr->mode & PNG_HAVE_IDAT) != 0)
png_ptr->mode |= PNG_AFTER_IDAT;
-#ifdef PNG_MAX_MALLOC_64K
- /* TODO: either remove this or common it out into png_read_buffer */
- if (length > 65535U)
- {
- png_crc_finish(png_ptr, length);
- png_chunk_benign_error(png_ptr, "too large to fit in memory");
- return handled_error;
- }
-#endif
-
- buffer = png_read_buffer(png_ptr, length+1, 1/*warn*/);
+ buffer = png_read_buffer(png_ptr, length+1);
if (buffer == NULL)
{
+ png_crc_finish(png_ptr, length);
png_chunk_benign_error(png_ptr, "out of memory");
return handled_error;
}
@@ -2570,16 +2486,15 @@
}
#endif
- /* TODO: this doesn't work (not after the above code) and shouldn't be
- * necessary, see the similar cases above.
- */
+ /* TODO: should not be necessary. */
if ((png_ptr->mode & PNG_HAVE_IDAT) != 0)
png_ptr->mode |= PNG_AFTER_IDAT;
/* Note, "length" is sufficient here; we won't be adding
- * a null terminator later.
+ * a null terminator later. The limit check in png_handle_chunk should be
+ * sufficient.
*/
- buffer = png_read_buffer(png_ptr, length, 2/*silent*/);
+ buffer = png_read_buffer(png_ptr, length);
if (buffer == NULL)
{
@@ -2691,13 +2606,11 @@
}
#endif
- /* TODO: this doesn't work (not after the above code) and shouldn't be
- * necessary, see the similar cases above.
- */
+ /* TODO: should not be necessary. */
if ((png_ptr->mode & PNG_HAVE_IDAT) != 0)
png_ptr->mode |= PNG_AFTER_IDAT;
- buffer = png_read_buffer(png_ptr, length+1, 1/*warn*/);
+ buffer = png_read_buffer(png_ptr, length+1);
if (buffer == NULL)
{
@@ -2822,7 +2735,7 @@
static int
png_cache_unknown_chunk(png_structrp png_ptr, png_uint_32 length)
{
- png_alloc_size_t limit = PNG_SIZE_MAX;
+ const png_alloc_size_t limit = png_chunk_max(png_ptr);
if (png_ptr->unknown_chunk.data != NULL)
{
@@ -2830,16 +2743,6 @@
png_ptr->unknown_chunk.data = NULL;
}
-# 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 (length <= limit)
{
PNG_CSTRING_FROM_CHUNK(png_ptr->unknown_chunk.name, png_ptr->chunk_name);
@@ -3112,7 +3015,12 @@
* build.
*/
- png_uint_32 max_length; /* Length min, max in bytes */
+ /* Crushing these values helps on modern 32-bit architectures because the
+ * pointer and the following bit fields both end up requiring 32 bits.
+ * Typically this will halve the table size. On 64-bit architectures the
+ * table entries will typically be 8 bytes.
+ */
+ png_uint_32 max_length :12; /* Length min, max in bytes */
png_uint_32 min_length :8;
/* Length errors on critical chunks have special handling to preserve the
* existing behaviour in libpng 1.6. Anciallary chunks are checked below
@@ -3135,47 +3043,63 @@
* the first, 'handler', function. 'handler' is NULL when the chunk has no
* compiled in support.
*/
-# define ChunkMax 0x7FFFFFFFU /* Maximum PNG chunk length */
-# define NoCheck ChunkMax /* Special check in handler */
-# define LKMin 3U+LZ77Min /* Minimum length of keyword+LZ77 */
+# define NoCheck 0x801U /* Do not check the maximum length */
+# define Limit 0x802U /* Limit to png_chunk_max bytes */
+# define LKMin 3U+LZ77Min /* Minimum length of keyword+LZ77 */
#define hIHDR PNG_HAVE_IHDR
#define hPLTE PNG_HAVE_PLTE
#define hIDAT PNG_HAVE_IDAT
+ /* For the two chunks, tRNS and bKGD which can occur in PNGs without a PLTE
+ * but must occur after the PLTE use this and put the check in the handler
+ * routine for colour mapped images were PLTE is required. Also put a check
+ * in PLTE for other image types to drop the PLTE if tRNS or bKGD have been
+ * seen.
+ */
#define hCOL (PNG_HAVE_PLTE|PNG_HAVE_IDAT)
+ /* Used for the decoding chunks which must be before PLTE. */
#define aIDAT PNG_AFTER_IDAT
/* Chunks from W3C PNG v3: */
/* cHNK max_len, min, before, after, multiple */
# define CDIHDR 13U, 13U, hIHDR, 0, 0
# define CDPLTE NoCheck, 0U, 0, hIHDR, 1
-# define CDIDAT ChunkMax, 0U, aIDAT, hIHDR, 1
+ /* PLTE errors are only critical for colour-map images, consequently the
+ * hander does all the checks.
+ */
+# define CDIDAT NoCheck, 0U, aIDAT, hIHDR, 1
# define CDIEND NoCheck, 0U, 0, aIDAT, 0
+ /* Historically data was allowed in IEND */
# define CDtRNS 256U, 0U, hIDAT, hIHDR, 0
# define CDcHRM 32U, 32U, hCOL, hIHDR, 0
# define CDgAMA 4U, 4U, hCOL, hIHDR, 0
-# define CDiCCP ChunkMax, LKMin, hCOL, hIHDR, 0
+# define CDiCCP NoCheck, LKMin, hCOL, hIHDR, 0
# define CDsBIT 4U, 1U, hCOL, hIHDR, 0
# define CDsRGB 1U, 1U, hCOL, hIHDR, 0
# define CDcICP 4U, 4U, hCOL, hIHDR, 0
# define CDmDCV 24U, 24U, hCOL, hIHDR, 0
-# define CDeXIf ChunkMax, 4U, 0, hIHDR, 0
+# define CDeXIf Limit, 4U, 0, hIHDR, 0
# define CDcLLI 8U, 8U, hCOL, hIHDR, 0
-# define CDtEXt ChunkMax, 2U, 0, hIHDR, 1
-# define CDzTXt ChunkMax, LKMin, 0, hIHDR, 1
-# define CDiTXt ChunkMax, 6U, 0, hIHDR, 1
+# define CDtEXt NoCheck, 2U, 0, hIHDR, 1
+ /* Allocates 'length+1'; checked in the handler */
+# define CDzTXt Limit, LKMin, 0, hIHDR, 1
+# define CDiTXt NoCheck, 6U, 0, hIHDR, 1
+ /* Allocates 'length+1'; checked in the handler */
# define CDbKGD 6U, 1U, hIDAT, hIHDR, 0
# define CDhIST 1024U, 0U, hPLTE, hIHDR, 0
-# define CDpHYs 9U, 9U, hPLTE, hIHDR, 0
-# define CDsPLT ChunkMax, 3U, hIDAT, hIHDR, 1
+# define CDpHYs 9U, 9U, hIDAT, hIHDR, 0
+# define CDsPLT NoCheck, 3U, hIDAT, hIHDR, 1
+ /* Allocates 'length+1'; checked in the handler */
# define CDtIME 7U, 7U, 0, hIHDR, 0
-# define CDacTL 8U, 8U, hPLTE, hIHDR, 0
+# define CDacTL 8U, 8U, hIDAT, hIHDR, 0
# define CDfcTL 25U, 26U, 0, hIHDR, 1
-# define CDfdAT ChunkMax, 4U, hPLTE, hIHDR, 0
- /* Supported chunks from PNG extensions 1.5.0 */
+# define CDfdAT Limit, 4U, hIDAT, hIHDR, 1
+ /* Supported chunks from PNG extensions 1.5.0, NYI so limit */
# define CDoFFs 9U, 9U, hIDAT, hIHDR, 0
-# define CDpCAL ChunkMax, 14U, hIDAT, hIHDR, 0
-# define CDsCAL ChunkMax, 4U, hIDAT, hIHDR, 0
+# define CDpCAL NoCheck, 14U, hIDAT, hIHDR, 0
+ /* Allocates 'length+1'; checked in the handler */
+# define CDsCAL Limit, 4U, hIDAT, hIHDR, 0
+ /* Allocates 'length+1'; checked in the handler */
# define PNG_CHUNK(cHNK, index) { png_handle_ ## cHNK, CD ## cHNK },
PNG_KNOWN_CHUNKS
@@ -3209,12 +3133,11 @@
/* CSE: these things don't change, these autos are just to save typing and
* make the code more clear.
*/
- const int chunk_critical = PNG_CHUNK_CRITICAL(png_ptr->chunk_name);
- const png_index chunk_index = png_chunk_index_from_name(png_ptr->chunk_name);
+ const png_uint_32 chunk_name = png_ptr->chunk_name;
+ const png_index chunk_index = png_chunk_index_from_name(chunk_name);
png_handle_result_code handled = handled_error;
png_const_charp errmsg = NULL;
- int critical_error = 0; /* default to 'benign' */
/* Is this a known chunk? If not there are no checks performed here;
* png_handle_unknown does the correct checks. This means that the values
@@ -3241,7 +3164,6 @@
read_chunks[chunk_index].pos_after))
{
errmsg = "out of place";
- critical_error = chunk_critical;
}
/* Now check for duplicates: duplicated critical chunks also produce a
@@ -3251,17 +3173,47 @@
png_file_has_chunk(png_ptr, chunk_index))
{
errmsg = "duplicate";
- critical_error = chunk_critical;
}
+ else if (length < read_chunks[chunk_index].min_length)
+ errmsg = "too short";
else
{
- if (length > read_chunks[chunk_index].max_length)
- errmsg = "too much data";
- else if (length < read_chunks[chunk_index].min_length)
- errmsg = "too little data";
- else
- handled = read_chunks[chunk_index].handler(png_ptr, info_ptr, length);
+ /* NOTE: apart from IHDR the critical chunks (PLTE, IDAT and IEND) are set
+ * up above not to do any length checks.
+ *
+ * The png_chunk_max check ensures that the variable length chunks are
+ * always checked at this point for being within the system allocation
+ * limits.
+ */
+ unsigned max_length = read_chunks[chunk_index].max_length;
+
+ switch (max_length)
+ {
+ case Limit:
+ /* png_read_chunk_header has already png_error'ed chunks with a
+ * length exceeding the 31-bit PNG limit, so just check the memory
+ * limit:
+ */
+ if (length <= png_chunk_max(png_ptr))
+ goto MeetsLimit;
+
+ errmsg = "length exceeds libpng limit";
+ break;
+
+ default:
+ if (length <= max_length)
+ goto MeetsLimit;
+
+ errmsg = "too long";
+ break;
+
+ case NoCheck:
+ MeetsLimit:
+ handled = read_chunks[chunk_index].handler(
+ png_ptr, info_ptr, length);
+ break;
+ }
}
/* If there was an error or the chunk was simply skipped it is not counted as
@@ -3269,9 +3221,9 @@
*/
if (errmsg != NULL)
{
- if (critical_error) /* stop immediately */
+ if (PNG_CHUNK_CRITICAL(chunk_name)) /* stop immediately */
png_chunk_error(png_ptr, errmsg);
- else
+ else /* ancillary chunk */
{
/* The chunk data is skipped: */
png_crc_finish(png_ptr, length);
@@ -4275,6 +4227,9 @@
avail_in = png_ptr->IDAT_read_size;
+ if (avail_in > png_chunk_max(png_ptr))
+ avail_in = png_chunk_max(png_ptr);
+
if (avail_in > png_ptr->idat_size)
avail_in = (uInt)png_ptr->idat_size;
@@ -4282,8 +4237,13 @@
* to minimize memory usage by causing lots of re-allocs, but
* realistically doing IDAT_read_size re-allocs is not likely to be a
* big problem.
+ *
+ * An error here corresponds to the system being out of memory.
*/
- buffer = png_read_buffer(png_ptr, avail_in, 0/*error*/);
+ buffer = png_read_buffer(png_ptr, avail_in);
+
+ if (buffer == NULL)
+ png_chunk_error(png_ptr, "out of memory");
png_crc_read(png_ptr, buffer, avail_in);
png_ptr->idat_size -= avail_in;
diff --git a/pngset.c b/pngset.c
index 329b5cb..d7f3393 100644
--- a/pngset.c
+++ b/pngset.c
@@ -1831,8 +1831,24 @@
{
png_debug(1, "in png_set_chunk_malloc_max");
+ /* pngstruct::user_chunk_malloc_max is initialized to a non-zero value in
+ * png.c. This API supports '0' for unlimited, make sure the correct
+ * (unlimited) value is set here to avoid a need to check for 0 everywhere
+ * the parameter is used.
+ */
if (png_ptr != NULL)
- png_ptr->user_chunk_malloc_max = user_chunk_malloc_max;
+ {
+ if (user_chunk_malloc_max == 0U) /* unlimited */
+ {
+# ifdef PNG_MAX_MALLOC_64K
+ png_ptr->user_chunk_malloc_max = 65536U;
+# else
+ png_ptr->user_chunk_malloc_max = PNG_SIZE_MAX;
+# endif
+ }
+ else
+ png_ptr->user_chunk_malloc_max = user_chunk_malloc_max;
+ }
}
#endif /* ?SET_USER_LIMITS */
