cheng/libpng
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[libpng16] Fixed a memory overwrite bug in simplified read of RGB PNG with

Browse Source 
Fixed a memory overwrite bug in simplified read of RGB PNG with
non-linear gamma Also bugs in the error checking in pngread.c and changed
quite a lot of the checks in pngstest.c to be correct; either correctly
written or not over-optimistic.  The pngstest changes are insufficient to
allow all possible RGB transforms to be passed; pngstest cmppixel needs
to be rewritten to make it clearer which errors it allows and then changed
to permit known inaccuracies.
This commit is contained in:
Glenn Randers-Pehrson 2012-02-10 17:13:13 -06:00
parent 74df4f1956
commit 864270e18f
4 changed files with 184 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ANNOUNCE
View File

@ -1,5 +1,5 @@
Libpng 1.6.0beta11 - February 4, 2012
Libpng 1.6.0beta11 - February 10, 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.
@ -176,7 +176,7 @@ Version 1.6.0beta10 [February 3, 2012]
Updated the prebuilt configure files to current condition.
Revised INSTALL information about autogen.sh; it works in tar distributions.
Version 1.6.0beta11 [February 4, 2012]
Version 1.6.0beta11 [February 10, 2012]
Fix character count in pngstest command in projects/owatcom/pngstest.tgt
Revised test-pngstest.sh to report PASS/FAIL for each image.
Updated documentation about the simplified API.
@ -188,6 +188,14 @@ Version 1.6.0beta11 [February 4, 2012]
error of 14.99/255; 0.5/255^(1/2.2). pngstest now uses 15 for the
permitted 8-bit error. This may still not be enough because of arithmetic
error.
Removed some unused arrays (with #ifdef) from png_read_push_finish_row().
Fixed a memory overwrite bug in simplified read of RGB PNG with
non-linear gamma Also bugs in the error checking in pngread.c and changed
quite a lot of the checks in pngstest.c to be correct; either correctly
written or not over-optimistic. The pngstest changes are insufficient to
allow all possible RGB transforms to be passed; pngstest cmppixel needs
to be rewritten to make it clearer which errors it allows and then changed
to permit known inaccuracies.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit

10
CHANGES
View File

@ -3927,7 +3927,7 @@ Version 1.6.0beta10 [February 3, 2012]
Updated the prebuilt configure files to current condition.
Revised INSTALL information about autogen.sh; it works in tar distributions.
Version 1.6.0beta11 [February 4, 2012]
Version 1.6.0beta11 [February 10, 2012]
Fix character count in pngstest command in projects/owatcom/pngstest.tgt
Revised test-pngstest.sh to report PASS/FAIL for each image.
Updated documentation about the simplified API.
@ -3939,6 +3939,14 @@ Version 1.6.0beta11 [February 4, 2012]
error of 14.99/255; 0.5/255^(1/2.2). pngstest now uses 15 for the
permitted 8-bit error. This may still not be enough because of arithmetic
error.
Removed some unused arrays (with #ifdef) from png_read_push_finish_row().
Fixed a memory overwrite bug in simplified read of RGB PNG with
non-linear gamma Also bugs in the error checking in pngread.c and changed
quite a lot of the checks in pngstest.c to be correct; either correctly
written or not over-optimistic. The pngstest changes are insufficient to
allow all possible RGB transforms to be passed; pngstest cmppixel needs
to be rewritten to make it clearer which errors it allows and then changed
to permit known inaccuracies.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit

245
contrib/libtests/pngstest.c
View File

@ -121,6 +121,19 @@ isRGB(int fixed_linear)
return sRGB(fixed_linear / 65535.);
}
static png_byte
unpremultiply(int component, int alpha)
{
if (alpha == 0)
return 255; /* Arbitrary, but consistent with the libpng code */
else if (alpha >= 65535)
return isRGB(component);
else
return sRGB((double)component / alpha);
}
static png_uint_16
ilineara(int fixed_srgb, int alpha)
{
@ -414,7 +427,7 @@ static void format_default(format_list *pf, int redundant)
if (redundant)
{
int i;
/* set everything, including flags that are pointless */
for (i=0; i<FORMAT_SET_COUNT; ++i)
pf->bits[i] = ~(png_uint_32)0;
@ -458,7 +471,6 @@ typedef struct
ptrdiff_t stride;
png_size_t bufsize;
png_size_t allocsize;
/* png_color background; */
char tmpfile_name[32];
png_uint_16 colormap[256*4];
}
@ -671,7 +683,7 @@ typedef struct
* The API returns false if an error is detected; this can only be if the alpha
* value is less than the component in the linear case.
*/
static int
static int
get_pixel(Image *image, Pixel *pixel, png_const_bytep pp)
{
png_uint_32 format = image->image.format;
@ -848,14 +860,89 @@ get_pixel(Image *image, Pixel *pixel, png_const_bytep pp)
static int error_to_linear = 811; /* by experiment */
static int error_to_linear_grayscale = 424; /* by experiment */
static int error_to_sRGB = 6; /* by experiment */
static int error_to_sRGB_grayscale = 15; /* libpng error by calculation */
static int error_in_compose = 0;
static int error_via_linear = 14; /* by experiment */
static int error_to_sRGB_grayscale = 17; /* libpng error by calculation +
2 by experiment */
static int error_in_compose = 2; /* by experiment */
static int error_in_premultiply = 1;
/* The following is *just* the result of a round trip from 8-bit sRGB to linear
* then back to 8-bit sRGB when it is done by libpng. There are two problems:
*
* 1) libpng currently uses a 2.2 power law with no linear segment, this results
* in instability in the low values and even with 16-bit precision sRGB(1) ends
* up mapping to sRGB(0) as a result of rounding in the 16-bit representation.
* This gives an error of 1 in the handling of value 1 only.
*
* 2) libpng currently uses an intermediate 8-bit linear value in gamma
* correction of 8-bit values. This results in many more errors, the worse of
* which is mapping sRGB(14) to sRGB(0).
*
* The general 'error_via_linear' is more complex because of pre-multiplication,
* this compounds the 8-bit errors according to the alpha value of the pixel.
* As a result 256 values are pre-calculated for error_via_linear.
*/
static int error_in_libpng_gamma;
static int error_via_linear[256]; /* Indexed by 8-bit alpha */
static void
init_error_via_linear(void)
{
int alpha;
error_via_linear[0] = 255; /* transparent pixel */
for (alpha=1; alpha<=255; ++alpha)
{
/* 16-bit values less than 128.5 get rounded to 8-bit 0 and so the worst
* case error arises with 16-bit 128.5, work out what sRGB
* (non-associated) value generates 128.5; any value less than this is
* going to map to 0, so the worst error is floor(value).
*
* Note that errors are considerably higher (more than a factor of 2)
* because libpng uses a simple power law for sRGB data at present.
*
* Add .1 for arithmetic errors inside libpng.
*/
double v = floor(255*pow(.5/*(128.5 * 255 / 65535)*/ / alpha, 1/2.2)+.1);
error_via_linear[alpha] = (int)v;
}
/* This is actually 14.99, but, despite the closeness to 15, 14 seems to work
* ok in this case.
*/
error_in_libpng_gamma = 14;
}
/* A precalculated floating point linear background value, for grayscale the
* green channel is used.
*/
typedef struct
{
double r, g, b;
}
precalc_background;
static void
init_background(precalc_background *ppb, const png_color *background)
{
if (background == NULL)
{
double v = linear_from_sRGB(BUFFER_INIT8/255.);
ppb->r = ppb->g = ppb->b = v;
}
else
{
ppb->r = linear_from_sRGB(background->red/255.);
ppb->g = linear_from_sRGB(background->green/255.);
ppb->b = linear_from_sRGB(background->blue/255.);
}
}
static const char *
cmppixel(Pixel *a, Pixel *b, const png_color *background, int via_linear,
int multiple_algorithms)
cmppixel(Pixel *a, Pixel *b, const precalc_background *background,
int via_linear, int multiple_algorithms)
{
int error_limit = 0;
@ -1120,63 +1207,25 @@ cmppixel(Pixel *a, Pixel *b, const png_color *background, int via_linear,
* see where the parameter is set to non-zero below.
*/
if (!(a->format & PNG_FORMAT_FLAG_LINEAR) && via_linear)
error_limit = error_via_linear;
if (b->format & PNG_FORMAT_FLAG_COLOR)
err = "8-bit color mismatch";
else
err = "8-bit gray mismatch";
/* If the original data had an alpha channel and was not pre-multiplied
* pre-multiplication may lose precision in non-opaque pixel values. If
* the output is linear the premultiplied 16-bit values will be used, but
* if 'via_linear' is set an intermediate 16-bit pre-multiplied form has
* been used and this must be taken into account here.
*/
if (via_linear && (a->format & PNG_FORMAT_FLAG_ALPHA) &&
!(a->format & PNG_FORMAT_FLAG_LINEAR) &&
a->a16 < 65535)
{
if (a->a16 > 0)
{
/* First calculate the rounded 16-bit component values, (r,g,b) or y
* as appropriate, then back-calculate the 8-bit values for
* comparison below.
*/
if (a->format & PNG_FORMAT_FLAG_COLOR)
{
double r = closestinteger((65535. * a->r16) / a->a16)/65535;
double g = closestinteger((65535. * a->g16) / a->a16)/65535;
double blue = closestinteger((65535. * a->b16) / a->a16)/65535;
a->r16 = u16d(r * a->a16);
a->g16 = u16d(g * a->a16);
a->b16 = u16d(blue * a->a16);
a->y16 = u16d(YfromRGBint(a->r16, a->g16, a->b16));
a->r8 = u8d(r * 255);
a->g8 = u8d(g * 255);
a->b8 = u8d(blue * 255);
a->y8 = u8d(255 * YfromRGB(r, g, blue));
}
else
{
double y = closestinteger((65535. * a->y16) / a->a16)/65535.;
a->b16 = a->g16 = a->r16 = a->y16 = u16d(y * a->a16);
a->b8 = a->g8 = a->r8 = a->y8 = u8d(255 * y);
}
}
if (a->a8 > 0)
error_limit = error_via_linear[a->a8];
else
{
a->r16 = a->g16 = a->b16 = a->y16 = 0;
a->r8 = a->g8 = a->b8 = a->y8 = 255;
/* The transform to linear will have set the component values to
* 65535, so they will come back as 255:
*/
error_limit = 0;
a->y8 = a->r8 = a->g8 = a->b8 = 255;
}
}
if (b->format & PNG_FORMAT_FLAG_COLOR)
err = "8-bit color mismatch";
else
err = "8-bit gray mismatch";
if (b->format & PNG_FORMAT_FLAG_ALPHA)
{
@ -1225,25 +1274,14 @@ cmppixel(Pixel *a, Pixel *b, const png_color *background, int via_linear,
{
if (b->format & PNG_FORMAT_FLAG_COLOR)
{
double r, g, blue;
r = (255. * b->r16)/b->a16;
b->r8 = u8d(r);
g = (255. * b->g16)/b->a16;
b->g8 = u8d(g);
blue = (255. * b->b16)/b->a16;
b->b8 = u8d(blue);
b->y8 = u8d(YfromRGB(r, g, blue));
b->r8 = unpremultiply(b->r16, b->a16);
b->g8 = unpremultiply(b->g16, b->a16);
b->b8 = unpremultiply(b->b16, b->a16);
b->y8 = unpremultiply(b->y16, b->a16);
}
else
{
b->r8 = b->g8 = b->b8 = b->y8 =
u8d((255. * b->y16)/b->a16);
}
b->r8 = b->g8 = b->b8 = b->y8 = unpremultiply(b->y16, b->a16);
}
else
@ -1279,45 +1317,25 @@ cmppixel(Pixel *a, Pixel *b, const png_color *background, int via_linear,
a->b8 = isRGB(a->b16);
a->y8 = isRGB(a->y16);
/* There should be no libpng error in this (ideally) */
error_limit = 0;
}
else if (background == NULL)
{
double add = alpha * linear_from_sRGB(BUFFER_INIT8/255.);
double r, g, blue, y;
r = a->r16 + add;
a->r16 = u16d(r);
a->r8 = sRGB(r/65535);
g = a->g16 + add;
a->g16 = u16d(g);
a->g8 = sRGB(g/65535);
blue = a->b16 + add;
a->b16 = u16d(blue);
a->b8 = sRGB(blue/65535);
y = YfromRGB(r, g, blue);
a->y16 = u16d(y);
a->y8 = sRGB(y/65535);
/* There should be no libpng error in this (ideally), but the
* libpng gamma correction code has off-by-one errors.
*/
error_limit = error_in_libpng_gamma;
}
else
{
double r, g, blue, y;
r = a->r16 + alpha * linear_from_sRGB(background->red/255.);
r = a->r16 + alpha * background->r;
a->r16 = u16d(r);
a->r8 = sRGB(r/65535);
g = a->g16 + alpha * linear_from_sRGB(background->green/255.);
g = a->g16 + alpha * background->g;
a->g16 = u16d(g);
a->g8 = sRGB(g/65535);
blue = a->b16 + alpha * linear_from_sRGB(background->blue/255.);
blue = a->b16 + alpha * background->b;
a->b16 = u16d(blue);
a->b8 = sRGB(blue/65535);
@ -1334,7 +1352,11 @@ cmppixel(Pixel *a, Pixel *b, const png_color *background, int via_linear,
if (via_linear)
{
a->r8 = a->g8 = a->b8 = a->y8 = isRGB(a->y16);
error_limit = 0;
/* There should be no libpng error in this (ideally), but the
* libpng gamma correction code has off-by-one errors.
*/
error_limit = error_in_libpng_gamma;
}
else
@ -1342,8 +1364,7 @@ cmppixel(Pixel *a, Pixel *b, const png_color *background, int via_linear,
/* When the output is gray the background comes from just the
* green channel.
*/
double y = a->y16 + alpha * linear_from_sRGB(
(background == NULL ? BUFFER_INIT8 : background->green)/255.);
double y = a->y16 + alpha * background->g;
a->r16 = a->g16 = a->b16 = a->y16 = u16d(y);
a->r8 = a->g8 = a->b8 = a->y8 = sRGB(y/65535);
@ -1557,6 +1578,7 @@ compare_two_images(Image *a, Image *b, int via_linear,
int two_algorithms = ((formata ^ formatb) & PNG_FORMAT_FLAG_COLORMAP) != 0;
int result = 1;
unsigned int check_alpha = 0; /* must be zero or one */
precalc_background pb;
png_byte swap_mask[4];
png_uint_32 x, y;
png_const_bytep ppa, ppb;
@ -1566,6 +1588,9 @@ compare_two_images(Image *a, Image *b, int via_linear,
return logerror(a, a->file_name, ": width x height changed: ",
b->file_name);
/* Set up the background */
init_background(&pb, background);
/* Find the first row and inter-row space. */
if (!(formata & PNG_FORMAT_FLAG_COLORMAP) &&
(formata & PNG_FORMAT_FLAG_LINEAR))
@ -1635,6 +1660,7 @@ compare_two_images(Image *a, Image *b, int via_linear,
{
/* Only 'b' has an alpha channel */
check_alpha = 1;
if (formatb & PNG_FORMAT_FLAG_AFIRST)
{
bstart = 1;
@ -1702,7 +1728,7 @@ compare_two_images(Image *a, Image *b, int via_linear,
return badpixel(b, entry, 0, &pixel_b,
"bad palette entry value");
if (cmppixel(&pixel_a, &pixel_b, background, via_linear,
if (cmppixel(&pixel_a, &pixel_b, &pb, via_linear,
0/*multiple_algorithms*/) != NULL)
break;
@ -1910,7 +1936,7 @@ compare_two_images(Image *a, Image *b, int via_linear,
while (x < width)
{
png_const_bytep colormap_a = (png_const_bytep)a->colormap;
switch (channels)
{
case 4:
@ -1948,7 +1974,7 @@ compare_two_images(Image *a, Image *b, int via_linear,
while (x < width)
{
png_const_bytep colormap_b = (png_const_bytep)b->colormap;
switch (channels)
{
case 4:
@ -2000,7 +2026,7 @@ compare_two_images(Image *a, Image *b, int via_linear,
if (!get_pixel(b, &pixel_b, ppb))
return badpixel(b, x, y, &pixel_b, "bad pixel value");
mismatch = cmppixel(&pixel_a, &pixel_b, background, via_linear,
mismatch = cmppixel(&pixel_a, &pixel_b, &pb, via_linear,
two_algorithms);
if (mismatch != NULL)
@ -2297,7 +2323,7 @@ testimage(Image *image, png_uint_32 opts, format_list *pf)
Image output;
newimage(&output);
result = 1;
/* Use the low bit of 'counter' to indicate whether or not to do alpha
@ -2419,6 +2445,7 @@ main(int argc, char **argv)
int retval = 0;
int c;
init_error_via_linear();
format_init(&formats);
for (c=1; c<argc; ++c)

44
pngread.c
View File

@ -443,6 +443,7 @@ png_read_row(png_structrp png_ptr, png_bytep row, png_bytep dsp_row)
return;
}
break;
case 5:
if ((png_ptr->row_number & 1) || png_ptr->width < 2)
{
@ -1981,7 +1982,7 @@ make_rgb_colormap(png_image_read_control *display)
#define PNG_RGB_INDEX(r,g,b) \
((png_byte)(6 * (6 * PNG_DIV51(r) + PNG_DIV51(g)) + PNG_DIV51(b)))
static int
static int
png_image_read_colormap(png_voidp argument)
{
png_image_read_control *display =
@ -2672,7 +2673,7 @@ png_image_read_colormap(png_voidp argument)
output_encoding);
}
}
else
png_create_colormap_entry(display, i, colormap[i].red,
colormap[i].green, colormap[i].blue,
@ -3070,7 +3071,7 @@ png_image_read_colormapped(png_voidp argument)
{
png_uint_32 y = image->height;
png_bytep row = display->first_row;
while (y-- > 0)
{
png_read_row(png_ptr, row, NULL);
@ -3230,14 +3231,14 @@ png_image_read_background(png_voidp argument)
if ((png_ptr->transformations & PNG_COMPOSE) != 0)
png_error(png_ptr, "unexpected compose");
/* The palette code zaps PNG_GAMMA in place... */
if ((png_ptr->color_type & PNG_COLOR_MASK_PALETTE) == 0 &&
(png_ptr->transformations & PNG_GAMMA) == 0)
png_error(png_ptr, "lost gamma correction");
if (png_get_channels(png_ptr, info_ptr) != 2)
png_error(png_ptr, "lost/gained channels");
/* Expect the 8-bit case to always remove the alpha channel */
if ((image->format & PNG_FORMAT_FLAG_LINEAR) == 0 &&
(image->format & PNG_FORMAT_FLAG_ALPHA) != 0)
png_error(png_ptr, "unexpected 8-bit transformation");
switch (png_ptr->interlaced)
{
case PNG_INTERLACE_NONE:
@ -3293,7 +3294,7 @@ png_image_read_background(png_voidp argument)
startx = 0;
stepx = stepy = 1;
}
if (display->background == NULL)
{
for (; y<height; y += stepy)
@ -3390,8 +3391,8 @@ png_image_read_background(png_voidp argument)
{
png_bytep first_row = display->first_row;
ptrdiff_t step_row = display->row_bytes;
unsigned int outchannels = png_get_channels(png_ptr, info_ptr);
int preserve_alpha = (image->format & PNG_FORMAT_FLAG_ALPHA) != 0;
unsigned int outchannels = 1+preserve_alpha;
int swap_alpha = 0;
if (preserve_alpha && (image->format & PNG_FORMAT_FLAG_AFIRST))
@ -3399,9 +3400,11 @@ png_image_read_background(png_voidp argument)
for (pass = 0; pass < passes; ++pass)
{
unsigned int startx, stepx, stepy; /* all in pixels */
unsigned int startx, stepx, stepy;
png_uint_32 y;
/* The 'x' start and step are adjusted to output components here.
*/
if (png_ptr->interlaced == PNG_INTERLACE_ADAM7)
{
/* The row may be empty for a short image: */
@ -3454,7 +3457,7 @@ png_image_read_background(png_voidp argument)
component = 0;
outrow[swap_alpha] = (png_uint_16)component;
if (outchannels > 1)
if (preserve_alpha)
outrow[1 ^ swap_alpha] = alpha;
inrow += 2; /* components and alpha channel */
@ -3489,7 +3492,7 @@ png_image_read_direct(png_voidp argument)
* need 8 bits minimum, no palette and expanded tRNS.
*/
png_set_expand(png_ptr);
/* Now check the format to see if it was modified. */
{
png_uint_32 base_format = png_image_format(png_ptr) &
@ -3800,10 +3803,21 @@ png_image_read_direct(png_voidp argument)
# endif
# ifdef PNG_FORMAT_AFIRST_SUPPORTED
if (png_ptr->transformations & PNG_SWAP_ALPHA ||
if (do_local_background == 2)
{
if (format & PNG_FORMAT_FLAG_AFIRST)
info_format |= PNG_FORMAT_FLAG_AFIRST;
}
if ((png_ptr->transformations & PNG_SWAP_ALPHA) != 0 ||
((png_ptr->transformations & PNG_ADD_ALPHA) != 0 &&
(png_ptr->flags & PNG_FLAG_FILLER_AFTER) == 0))
{
if (do_local_background == 2)
png_error(png_ptr, "unexpected alpha swap transformation");
info_format |= PNG_FORMAT_FLAG_AFIRST;
}
# endif
/* This is actually an internal error. */
@ -3869,7 +3883,7 @@ png_image_read_direct(png_voidp argument)
{
png_uint_32 y = image->height;
png_bytep row = display->first_row;
while (y-- > 0)
{
png_read_row(png_ptr, row, NULL);