Skip to content

Commit

Permalink
fix: png_write_iCCP check on profile length
Browse files Browse the repository at this point in the history
This is a regression of commit a8242dd
"PNGv3 colourspace precedence rules conformance".

Previously, `png_write_iCCP` used the length from the first four bytes
of the profile set by `png_set_iCCP`, rather than the actual data length
recorded by `png_set_iCCP`.

If the profile data were less than 4 bytes long, it would have caused
a read-beyond-end-of-malloc error.  This bug was in the libpng code even
before the changes introduced in the above-mentioned commit, but it was
inaccessible.  It became accessible when we removed the pre-PNGv3 colour
space checks in `png_set_iCCP`.

Reported-by: Bob Friesenhahn <bobjfriesenhahn@gmail.com>
Reviewed-by: Cosmin Truta <ctruta@gmail.com>
Signed-off-by: John Bowler <jbowler@acm.org>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>
  • Loading branch information
jbowler authored and ctruta committed Jan 30, 2025
1 parent 8c7ed2e commit 68e090e
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
8 changes: 4 additions & 4 deletions pngpriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -1276,10 +1276,10 @@ PNG_INTERNAL_FUNCTION(void,png_write_eXIf,(png_structrp png_ptr,

#ifdef PNG_WRITE_iCCP_SUPPORTED
PNG_INTERNAL_FUNCTION(void,png_write_iCCP,(png_structrp png_ptr,
png_const_charp name, png_const_bytep profile), PNG_EMPTY);
/* The profile must have been previously validated for correctness, the
* length comes from the first four bytes. Only the base, deflate,
* compression is supported.
png_const_charp name, png_const_bytep profile, png_uint_32 proflen),
PNG_EMPTY);
/* Writes a previously 'set' profile. The profile argument is **not**
* compressed.
*/
#endif

Expand Down
2 changes: 1 addition & 1 deletion pngwrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ png_write_info_before_PLTE(png_structrp png_ptr, png_const_inforp info_ptr)
if ((info_ptr->valid & PNG_INFO_iCCP) != 0)
{
png_write_iCCP(png_ptr, info_ptr->iccp_name,
info_ptr->iccp_profile);
info_ptr->iccp_profile, info_ptr->iccp_proflen);
}
# endif

Expand Down
8 changes: 4 additions & 4 deletions pngwutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -1132,10 +1132,9 @@ png_write_sRGB(png_structrp png_ptr, int srgb_intent)
/* Write an iCCP chunk */
void /* PRIVATE */
png_write_iCCP(png_structrp png_ptr, png_const_charp name,
png_const_bytep profile)
png_const_bytep profile, png_uint_32 profile_len)
{
png_uint_32 name_len;
png_uint_32 profile_len;
png_byte new_name[81]; /* 1 byte for the compression byte */
compression_state comp;
png_uint_32 temp;
Expand All @@ -1148,11 +1147,12 @@ png_write_iCCP(png_structrp png_ptr, png_const_charp name,
if (profile == NULL)
png_error(png_ptr, "No profile for iCCP chunk"); /* internal error */

profile_len = png_get_uint_32(profile);

if (profile_len < 132)
png_error(png_ptr, "ICC profile too short");

if (png_get_uint_32(profile) != profile_len)
png_error(png_ptr, "Incorrect data in iCCP");

temp = (png_uint_32) (*(profile+8));
if (temp > 3 && (profile_len & 0x03))
png_error(png_ptr, "ICC profile length invalid (not a multiple of 4)");
Expand Down

0 comments on commit 68e090e

Please sign in to comment.