[libpng16] Fixed a memory overwrite bug in simplified read of RGB PNG with
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.
diff --git a/ANNOUNCE b/ANNOUNCE
index 7ee4310..7756606 100644
--- a/ANNOUNCE
+++ b/ANNOUNCE
@@ -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 @@
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 @@
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
diff --git a/CHANGES b/CHANGES
index 4fb9111..3342c73 100644
--- a/CHANGES
+++ b/CHANGES
@@ -3927,7 +3927,7 @@
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 @@
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
diff --git a/contrib/libtests/pngstest.c b/contrib/libtests/pngstest.c
index bc42dfb..74e91dc 100644
--- a/contrib/libtests/pngstest.c
+++ b/contrib/libtests/pngstest.c
@@ -121,6 +121,19 @@
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 @@
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 @@
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 @@
* 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 @@
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 @@
* 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 @@
{
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 @@
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 @@
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 @@
/* 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 @@
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 @@
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 @@
{
/* Only 'b' has an alpha channel */
check_alpha = 1;
+
if (formatb & PNG_FORMAT_FLAG_AFIRST)
{
bstart = 1;
@@ -1702,7 +1728,7 @@
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 @@
while (x < width)
{
png_const_bytep colormap_a = (png_const_bytep)a->colormap;
-
+
switch (channels)
{
case 4:
@@ -1948,7 +1974,7 @@
while (x < width)
{
png_const_bytep colormap_b = (png_const_bytep)b->colormap;
-
+
switch (channels)
{
case 4:
@@ -2000,7 +2026,7 @@
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 @@
Image output;
newimage(&output);
-
+
result = 1;
/* Use the low bit of 'counter' to indicate whether or not to do alpha
@@ -2419,6 +2445,7 @@
int retval = 0;
int c;
+ init_error_via_linear();
format_init(&formats);
for (c=1; c<argc; ++c)
diff --git a/pngread.c b/pngread.c
index c4cc05a..4560ca9 100644
--- a/pngread.c
+++ b/pngread.c
@@ -443,6 +443,7 @@
return;
}
break;
+
case 5:
if ((png_ptr->row_number & 1) || png_ptr->width < 2)
{
@@ -1981,7 +1982,7 @@
#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 @@
output_encoding);
}
}
-
+
else
png_create_colormap_entry(display, i, colormap[i].red,
colormap[i].green, colormap[i].blue,
@@ -3070,7 +3071,7 @@
{
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 @@
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 @@
startx = 0;
stepx = stepy = 1;
}
-
+
if (display->background == NULL)
{
for (; y<height; y += stepy)
@@ -3390,8 +3391,8 @@
{
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 @@
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 @@
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 @@
* 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 @@
# 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_uint_32 y = image->height;
png_bytep row = display->first_row;
-
+
while (y-- > 0)
{
png_read_row(png_ptr, row, NULL);
