[libpng16] Cleaned up and corrected ICC profile handling.

contrib/libtests/makepng: corrected 'rgb' and 'gray' cases.  profile_error
    messages could be truncated; made a correct buffer size calculation and
    adjusted pngerror.c appropriately. png_icc_check_* checking improved;
    changed the functions to receive the correct color type of the PNG on read
    or write and check that it matches the color space of the profile (despite
    what the comments said before, there is danger in assuming the app will
    cope correctly with an RGB profile on a grayscale image and, since it
    violates the PNG spec, allowing it is certain to produce inconsistent
    app behavior and might even cause app crashes.) Check that profiles
    contain the tags needed to process the PNG (tags all required by the ICC
    spec). Removed unused PNG_STATIC from pngpriv.h.
This commit is contained in:
John Bowler 2012-08-25 16:21:46 -05:00 committed by Glenn Randers-Pehrson
parent 8010217201
commit 14d0ca620e
8 changed files with 185 additions and 45 deletions

View File

@ -1,5 +1,5 @@
Libpng 1.6.0beta28 - August 18, 2012
Libpng 1.6.0beta28 - August 25, 2012
This is not intended to be a public release. It will be replaced
within a few weeks by a public version or by another test version.
@ -442,7 +442,7 @@ Version 1.6.0beta27 [August 11, 2012]
Work around gcc 3.x and Microsoft Visual Studio 2010 complaints. Both object
to the split initialization of num_chunks.
Version 1.6.0beta28 [August 18, 2012]
Version 1.6.0beta28 [August 25, 2012]
Unknown handling fixes and clean up. This adds more correct option
control of the unknown handling, corrects the pre-existing bug where
the per-chunk 'keep' setting is ignored and makes it possible to skip
@ -469,6 +469,18 @@ Version 1.6.0beta28 [August 18, 2012]
SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set
a user callback an unknown chunk will not be read, leading to a read
error, which was revealed by the "tunknown" test.
Cleaned up and corrected ICC profile handling.
contrib/libtests/makepng: corrected 'rgb' and 'gray' cases. profile_error
messages could be truncated; made a correct buffer size calculation and
adjusted pngerror.c appropriately. png_icc_check_* checking improved;
changed the functions to receive the correct color type of the PNG on read
or write and check that it matches the color space of the profile (despite
what the comments said before, there is danger in assuming the app will
cope correctly with an RGB profile on a grayscale image and, since it
violates the PNG spec, allowing it is certain to produce inconsistent
app behavior and might even cause app crashes.) Check that profiles
contain the tags needed to process the PNG (tags all required by the ICC
spec). Removed unused PNG_STATIC from pngpriv.h.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit

14
CHANGES
View File

@ -4193,7 +4193,7 @@ Version 1.6.0beta27 [August 11, 2012]
Work around gcc 3.x and Microsoft Visual Studio 2010 complaints. Both object
to the split initialization of num_chunks.
Version 1.6.0beta28 [August 18, 2012]
Version 1.6.0beta28 [August 25, 2012]
Unknown handling fixes and clean up. This adds more correct option
control of the unknown handling, corrects the pre-existing bug where
the per-chunk 'keep' setting is ignored and makes it possible to skip
@ -4220,6 +4220,18 @@ Version 1.6.0beta28 [August 18, 2012]
SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set
a user callback an unknown chunk will not be read, leading to a read
error, which was revealed by the "tunknown" test.
Cleaned up and corrected ICC profile handling.
contrib/libtests/makepng: corrected 'rgb' and 'gray' cases. profile_error
messages could be truncated; made a correct buffer size calculation and
adjusted pngerror.c appropriately. png_icc_check_* checking improved;
changed the functions to receive the correct color type of the PNG on read
or write and check that it matches the color space of the profile (despite
what the comments said before, there is danger in assuming the app will
cope correctly with an RGB profile on a grayscale image and, since it
violates the PNG spec, allowing it is certain to produce inconsistent
app behavior and might even cause app crashes.) Check that profiles
contain the tags needed to process the PNG (tags all required by the ICC
spec). Removed unused PNG_STATIC from pngpriv.h.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit

View File

@ -72,6 +72,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <math.h>
#include <errno.h>
@ -1125,7 +1126,7 @@ main(int argc, char **argv)
if (strncmp(arg, "gray", 4) == 0)
{
if (arg[5] == 0)
if (arg[4] == 0)
{
color_type = PNG_COLOR_TYPE_GRAY;
continue;
@ -1142,7 +1143,7 @@ main(int argc, char **argv)
if (strncmp(arg, "rgb", 3) == 0)
{
if (arg[4] == 0)
if (arg[3] == 0)
{
color_type = PNG_COLOR_TYPE_RGB;
continue;
@ -1157,7 +1158,7 @@ main(int argc, char **argv)
}
}
if (color_type == 8)
if (color_type == 8 && isdigit(arg[0]))
{
color_type = atoi(arg);
if (color_type < 0 || color_type > 6 || color_type == 1 ||
@ -1170,7 +1171,7 @@ main(int argc, char **argv)
continue;
}
if (bit_depth == 32)
if (bit_depth == 32 && isdigit(arg[0]))
{
bit_depth = atoi(arg);
if (bit_depth <= 0 || bit_depth > 16 ||

165
png.c
View File

@ -749,13 +749,13 @@ png_get_copyright(png_const_structrp png_ptr)
#else
# ifdef __STDC__
return PNG_STRING_NEWLINE \
"libpng version 1.6.0beta28 - August 22, 2012" PNG_STRING_NEWLINE \
"libpng version 1.6.0beta28 - August 25, 2012" PNG_STRING_NEWLINE \
"Copyright (c) 1998-2012 Glenn Randers-Pehrson" PNG_STRING_NEWLINE \
"Copyright (c) 1996-1997 Andreas Dilger" PNG_STRING_NEWLINE \
"Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc." \
PNG_STRING_NEWLINE;
# else
return "libpng version 1.6.0beta28 - August 22, 2012\
return "libpng version 1.6.0beta28 - August 25, 2012\
Copyright (c) 1998-2012 Glenn Randers-Pehrson\
Copyright (c) 1996-1997 Andreas Dilger\
Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.";
@ -1693,24 +1693,25 @@ profile_error(png_const_structrp png_ptr, png_colorspacerp colorspace,
png_const_charp name, png_alloc_size_t value, png_const_charp reason)
{
size_t pos;
char message[256];
char message[196]; /* see below for calculation */
if (colorspace != NULL)
colorspace->flags |= PNG_COLORSPACE_INVALID;
pos = png_safecat(message, (sizeof message), 0, "profile '");
pos = png_safecat(message, pos+79, pos, name);
pos = png_safecat(message, (sizeof message), pos, "': ");
pos = png_safecat(message, (sizeof message), 0, "profile '"); /* 9 chars */
pos = png_safecat(message, pos+79, pos, name); /* Truncate to 79 chars */
pos = png_safecat(message, (sizeof message), pos, "': "); /* +2 = 90 */
# ifdef PNG_WARNINGS_SUPPORTED
{
char number[PNG_NUMBER_BUFFER_SIZE];
char number[PNG_NUMBER_BUFFER_SIZE]; /* +24 = 114*/
pos = png_safecat(message, (sizeof message), pos,
png_format_number(number, number+(sizeof number),
PNG_NUMBER_FORMAT_x, value));
}
pos = png_safecat(message, (sizeof message), pos, ": ");
pos = png_safecat(message, (sizeof message), pos, "h: "); /* +2 = 116 */
# endif
/* The 'reason' is an arbitrary message, allow +79 maximum 195 */
pos = png_safecat(message, (sizeof message), pos, reason);
if (colorspace != NULL)
@ -1874,7 +1875,7 @@ png_icc_check_length(png_const_structrp png_ptr, png_colorspacerp colorspace,
int /* PRIVATE */
png_icc_check_header(png_const_structrp png_ptr, png_colorspacerp colorspace,
png_const_charp name, png_uint_32 profile_length,
png_const_bytep profile/* first 132 bytes only */)
png_const_bytep profile/* first 132 bytes only */, int color_type)
{
png_uint_32 temp;
@ -1930,16 +1931,37 @@ png_icc_check_header(png_const_structrp png_ptr, png_colorspacerp colorspace,
* 6), or a greyscale colour space for greyscale images (PNG colour types 0
* and 4)."
*
* This code does not check the color type because png_set_iCCP may be called
* before png_set_IHDR on write and because, anyway, the PNG spec is
* fundamentally flawed: RGB profiles can be used quite meaningfully for
* grayscale images and both RGB and palette images might only have gray
* colors in them, so gray profiles may be appropriate.
* This checking code ensures the embedded profile (on either read or write)
* conforms to the specification requirements. Notice that an ICC 'gray'
* color-space profile contains the information to transform the monochrome
* data to XYZ or L*a*b (according to which PCS the profile uses) and this
* should be used in preference to the standard libpng K channel replication
* into R, G and B channels.
*
* Previously it was suggested that an RGB profile on grayscale data could be
* handled. However it it is clear that using an RGB profile in this context
* must be an error - there is no specification of what it means. Thus it is
* almost certainly more correct to ignore the profile.
*/
temp = png_get_uint_32(profile+16); /* data colour space field */
if (temp != 0x52474220 /* 'RGB ' */ && temp != 0x47524159 /* 'GRAY' */)
return profile_error(png_ptr, colorspace, name, temp,
"invalid color space");
switch (temp)
{
case 0x52474220: /* 'RGB ' */
if (!(color_type & PNG_COLOR_MASK_COLOR))
return profile_error(png_ptr, colorspace, name, temp,
"RGB color space not permitted on grayscale PNG");
break;
case 0x47524159: /* 'GRAY' */
if (color_type & PNG_COLOR_MASK_COLOR)
return profile_error(png_ptr, colorspace, name, temp,
"Gray color space not permitted on RGB PNG");
break;
default:
return profile_error(png_ptr, colorspace, name, temp,
"invalid color space");
}
/* It is up to the application to check that the profile class matches the
* application requirements; the spec provides no guidance, but it's pretty
@ -1956,13 +1978,16 @@ png_icc_check_header(png_const_structrp png_ptr, png_colorspacerp colorspace,
{
if (temp == 0x6C696E6B /* 'link' */ || temp == 0x61627374 /* 'abst' */)
return profile_error(png_ptr, colorspace, name, temp,
"invalid class");
"invalid ICC profile class");
/* This can only be 0x6E6D636C: a 'nmcl' profile. This is a device
* specific profile.
* specific profile. The checks on the tags below will ensure that it can
* actually be used, but it certainly is not expected and is probably an
* error.
*/
else
(void)profile_error(png_ptr, NULL, name, temp, "unexpected ICC class");
(void)profile_error(png_ptr, NULL, name, temp,
"unexpected ICC profile class");
}
return 1;
@ -1976,6 +2001,16 @@ png_icc_check_tag_table(png_const_structrp png_ptr, png_colorspacerp colorspace,
png_uint_32 tag_count = png_get_uint_32(profile+128);
png_uint_32 itag;
png_const_bytep tag = profile+132; /* The first tag */
int have_AToB0Tag = 0; /* Whether the profile has an AToB0Tag */
int have_grayTRCTag = 0; /* Whether the profile has a grayTRCTag */
unsigned int matrix_TRC_tags = 0; /* Which matrix/TRC tags are present */
# define HAVE_redMatrixColumnTag 0x01
# define HAVE_greenMatrixColumnTag 0x02
# define HAVE_blueMatrixColumnTag 0x04
# define HAVE_redTRCTag 0x10
# define HAVE_greenTRCTag 0x20
# define HAVE_blueTRCTag 0x40
# define HAVE_all_tags 0x77
for (itag=0; itag < tag_count; ++itag, tag += 12)
{
@ -1992,9 +2027,90 @@ png_icc_check_tag_table(png_const_structrp png_ptr, png_colorspacerp colorspace,
tag_length > profile_length - tag_start)
return profile_error(png_ptr, colorspace, name, tag_id,
"tag data outside profile");
/* Check the tag_id for the specific profiles which must be present for
* the profile to be valid.
*/
switch (tag_id)
{
case 0x41324230: /* 'A2B0' - AToB0Tag */
have_AToB0Tag = 1;
break;
case 0x6B545243: /* 'kTRC' - grayTRCTag */
have_grayTRCTag = 1;
break;
case 0x7258595A: /* 'rXYZ' - redMatrixColumnTag */
matrix_TRC_tags |= HAVE_redMatrixColumnTag;
break;
case 0x72545243: /* 'rTRC' - redTRCTag */
matrix_TRC_tags |= HAVE_redTRCTag;
break;
case 0x6758595A: /* 'gXYZ' - greenMatrixColumnTag */
matrix_TRC_tags |= HAVE_greenMatrixColumnTag;
break;
case 0x67545243: /* 'gTRC' - greenTRCTag */
matrix_TRC_tags |= HAVE_greenTRCTag;
break;
case 0x6258595A: /* 'bXYZ' - blueMatrixColumnTag */
matrix_TRC_tags |= HAVE_blueMatrixColumnTag;
break;
case 0x62545243: /* 'bTRC' - blueTRCTag */
matrix_TRC_tags |= HAVE_blueTRCTag;
break;
default:
break;
}
}
return 1;
/* An AToB0Tag works in all valid profiles, but if it is absent then
* something matching the profile class and color space must be present.
*/
if (!have_AToB0Tag)
{
png_uint_32 profile_class = png_get_uint_32(profile+12);
switch (profile_class)
{
case 0x73636E72: /* 'scnr' - an input profile */
case 0x6D6E7472: /* 'mntr' - a display device profile */
case 0x70727472: /* 'prtr' - an output device profile */
if (png_get_uint_32(profile+16) /* color space */ ==
0x47524159 /* gray */)
{
if (!have_grayTRCTag)
return profile_error(png_ptr, colorspace, name, profile_class,
"missing grayTRCTag for monochrome profile");
}
else
{
if (matrix_TRC_tags != HAVE_all_tags)
return profile_error(png_ptr, colorspace, name, profile_class,
"missing Matrix/TRC tags for RGB profile");
}
return 1;
case 0x73706163: /* 'spac' */
return profile_error(png_ptr, colorspace, name, profile_class,
"missing AToB0Tag for colorspace profile");
default: /* should have been checked before */
png_error(png_ptr, "invalid ICC class");
return 0; /* NOT REACHED */
}
}
else
return 1;
}
#ifdef PNG_sRGB_SUPPORTED
@ -2192,14 +2308,15 @@ png_icc_set_gAMA_and_cHRM(png_const_structrp png_ptr,
int /* PRIVATE */
png_colorspace_set_ICC(png_const_structrp png_ptr, png_colorspacerp colorspace,
png_const_charp name, png_uint_32 profile_length,
png_const_bytep profile, int preferred)
png_const_bytep profile, int preferred, int color_type)
{
if (colorspace->flags & PNG_COLORSPACE_INVALID)
return 0;
if (png_icc_check_length(png_ptr, colorspace, name, profile_length) &&
png_icc_check_header(png_ptr, colorspace, name, profile_length, profile)
&& png_icc_check_tag_table(png_ptr, colorspace, name, profile_length,
png_icc_check_header(png_ptr, colorspace, name, profile_length, profile,
color_type) &&
png_icc_check_tag_table(png_ptr, colorspace, name, profile_length,
profile))
{
png_icc_set_gAMA_and_cHRM(png_ptr, colorspace, name, profile, 0,

View File

@ -415,7 +415,7 @@ static PNG_CONST char png_digit[16] = {
'A', 'B', 'C', 'D', 'E', 'F'
};
#define PNG_MAX_ERROR_TEXT 64
#define PNG_MAX_ERROR_TEXT 196 /* Currently limited be profile_error in png.c */
#if defined(PNG_WARNINGS_SUPPORTED) || defined(PNG_ERROR_TEXT_SUPPORTED)
static void /* PRIVATE */
png_format_buffer(png_const_structrp png_ptr, png_charp buffer, png_const_charp

View File

@ -281,13 +281,6 @@ typedef const png_uint_16p * png_const_uint_16pp;
# define PNG_ZBUF_SIZE 65536L
#endif
/* PNG_STATIC is used to mark internal file scope functions if they need to be
* accessed for implementation tests (see the code in tests/?*).
*/
#ifndef PNG_STATIC
# define PNG_STATIC static
#endif
/* If warnings or errors are turned off the code is disabled or redirected here.
* From 1.5.4 functions have been added to allow very limited formatting of
* error and warning messages - this code will also be disabled here.
@ -1449,8 +1442,8 @@ PNG_INTERNAL_FUNCTION(int,png_colorspace_set_sRGB,(png_const_structrp png_ptr,
#ifdef PNG_iCCP_SUPPORTED
PNG_INTERNAL_FUNCTION(int,png_colorspace_set_ICC,(png_const_structrp png_ptr,
png_colorspacerp colorspace, png_const_charp name,
png_uint_32 profile_length, png_const_bytep profile, int preferred),
PNG_EMPTY);
png_uint_32 profile_length, png_const_bytep profile, int preferred,
int color_type), PNG_EMPTY);
/* The 'name' is used for information only */
/* Routines for checking parts of an ICC profile. */
@ -1460,7 +1453,8 @@ PNG_INTERNAL_FUNCTION(int,png_icc_check_length,(png_const_structrp png_ptr,
PNG_INTERNAL_FUNCTION(int,png_icc_check_header,(png_const_structrp png_ptr,
png_colorspacerp colorspace, png_const_charp name,
png_uint_32 profile_length,
png_const_bytep profile /* first 132 bytes only */), PNG_EMPTY);
png_const_bytep profile /* first 132 bytes only */, int color_type),
PNG_EMPTY);
PNG_INTERNAL_FUNCTION(int,png_icc_check_tag_table,(png_const_structrp png_ptr,
png_colorspacerp colorspace, png_const_charp name,
png_uint_32 profile_length,

View File

@ -1424,7 +1424,8 @@ png_handle_iCCP(png_structrp png_ptr, png_inforp info_ptr, png_uint_32 length)
* byte header.
*/
if (png_icc_check_header(png_ptr, &png_ptr->colorspace,
keyword, profile_length, profile_header))
keyword, profile_length, profile_header,
png_ptr->color_type))
{
/* Now read the tag table; a variable size buffer is
* needed at this point, allocate one for the whole

View File

@ -643,11 +643,14 @@ png_set_iCCP(png_const_structrp png_ptr, png_inforp info_ptr,
/* Set the colorspace first because this validates the profile; do not
* override previously set app cHRM or gAMA here (because likely as not the
* application knows better than libpng what the correct values are.)
* application knows better than libpng what the correct values are.) Pass
* the info_ptr color_type field to png_colorspace_set_ICC because in the
* write case it has not yet been stored in png_ptr.
*/
{
int result = png_colorspace_set_ICC(png_ptr, &info_ptr->colorspace, name,
proflen, profile, 0/* do *not* override the app cHRM or gAMA */);
proflen, profile, 0/* do *not* override the app cHRM or gAMA */,
info_ptr->color_type);
png_colorspace_sync_info(png_ptr, info_ptr);