From d2f0bc2d13e6923c043d76c1d99ad2c532d78ada Mon Sep 17 00:00:00 2001 From: John Bowler Date: Sat, 11 Jun 2011 06:42:06 -0500 Subject: [PATCH] [devel] Improved gamma range checks and other things OpenWatcom warns about. --- ANNOUNCE | 1 + CHANGES | 1 + png.c | 29 ++++++++++++----------------- pngset.c | 21 +++++++++++---------- pngvalid.c | 6 +++--- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index 2b9e47233..be24b8d6d 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -169,6 +169,7 @@ Version 1.5.3beta11 [June 11, 2011] IDE autogenerates appropriate makefiles (libpng.mk) for batch processing. The project is configurable, unlike the Visual Studio project, so long as the developer has an awk. + Improved gamma range checks and other things OpenWatcom warns about. Send comments/corrections/commendations to png-mng-implement at lists.sf.net: (subscription required; visit diff --git a/CHANGES b/CHANGES index 4af577943..e1f8d4976 100644 --- a/CHANGES +++ b/CHANGES @@ -3432,6 +3432,7 @@ Version 1.5.3beta11 [June 11, 2011] IDE autogenerates appropriate makefiles (libpng.mk) for batch processing. The project is configurable, unlike the Visual Studio project, so long as the developer has an awk. + Improved gamma range checks and other things OpenWatcom warns about. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/png.c b/png.c index 0dab2264d..bf31bddd2 100644 --- a/png.c +++ b/png.c @@ -743,6 +743,13 @@ png_check_cHRM_fixed(png_structp png_ptr, if (png_ptr == NULL) return 0; + /* (x,y,z) values are first limited to 0..100000 (PNG_FP_1), the white + * y must also be greater than 0. To test for the upper limit calculate + * (PNG_FP_1-y) - x must be <= to this for z to be >= 0 (and the expression + * cannot overflow.) At this point we know x and y are >= 0 and (x+y) is + * <= PNG_FP_1. The previous test on PNG_MAX_UINT_31 is removed because it + * pointless (and it produces compiler warnings!) + */ if (white_x < 0 || white_y <= 0 || red_x < 0 || red_y < 0 || green_x < 0 || green_y < 0 || @@ -752,38 +759,26 @@ png_check_cHRM_fixed(png_structp png_ptr, "Ignoring attempt to set negative chromaticity value"); ret = 0; } - if (white_x > (png_fixed_point)PNG_UINT_31_MAX || - white_y > (png_fixed_point)PNG_UINT_31_MAX || - red_x > (png_fixed_point)PNG_UINT_31_MAX || - red_y > (png_fixed_point)PNG_UINT_31_MAX || - green_x > (png_fixed_point)PNG_UINT_31_MAX || - green_y > (png_fixed_point)PNG_UINT_31_MAX || - blue_x > (png_fixed_point)PNG_UINT_31_MAX || - blue_y > (png_fixed_point)PNG_UINT_31_MAX ) - { - png_warning(png_ptr, - "Ignoring attempt to set chromaticity value exceeding 21474.83"); - ret = 0; - } - if (white_x > 100000L - white_y) + /* And (x+y) must be <= PNG_FP_1 (so z is >= 0) */ + if (white_x > PNG_FP_1 - white_y) { png_warning(png_ptr, "Invalid cHRM white point"); ret = 0; } - if (red_x > 100000L - red_y) + if (red_x > PNG_FP_1 - red_y) { png_warning(png_ptr, "Invalid cHRM red point"); ret = 0; } - if (green_x > 100000L - green_y) + if (green_x > PNG_FP_1 - green_y) { png_warning(png_ptr, "Invalid cHRM green point"); ret = 0; } - if (blue_x > 100000L - blue_y) + if (blue_x > PNG_FP_1 - blue_y) { png_warning(png_ptr, "Invalid cHRM blue point"); ret = 0; diff --git a/pngset.c b/pngset.c index 2c58f7fc7..5fe984120 100644 --- a/pngset.c +++ b/pngset.c @@ -94,15 +94,16 @@ png_set_gAMA_fixed(png_structp png_ptr, png_infop info_ptr, png_fixed_point if (png_ptr == NULL || info_ptr == NULL) return; - /* Previously these values were limited, however they must be - * wrong, therefore storing them (and setting PNG_INFO_gAMA) - * must be wrong too. + /* Changed in libpng-1.5.4 to limit the values to ensure overflow can't + * occur. Since the fixed point representation is assymetrical it is + * possible for 1/gamma to overflow the limit of 21474 and this means the + * gamma value must be at least 5/100000 and hence at most 20000.0. For + * safety the limits here are a little narrower. The values are 0.00016 to + * 6250.0, which are truely ridiculous gammma values (and will produce + * displays that are all black or all white.) */ - if (file_gamma > (png_fixed_point)PNG_UINT_31_MAX) - png_warning(png_ptr, "Gamma too large, ignored"); - - else if (file_gamma <= 0) - png_warning(png_ptr, "Negative or zero gamma ignored"); + if (file_gamma < 16 || file_gamma > 625000000) + png_warning(png_ptr, "Out of range gamma value ignored"); else { @@ -340,11 +341,11 @@ png_set_sCAL_s(png_structp png_ptr, png_infop info_ptr, if (unit != 1 && unit != 2) png_error(png_ptr, "Invalid sCAL unit"); - if (swidth == NULL || (lengthw = png_strlen(swidth)) <= 0 || + if (swidth == NULL || (lengthw = png_strlen(swidth)) == 0 || swidth[0] == 45 /* '-' */ || !png_check_fp_string(swidth, lengthw)) png_error(png_ptr, "Invalid sCAL width"); - if (sheight == NULL || (lengthh = png_strlen(sheight)) <= 0 || + if (sheight == NULL || (lengthh = png_strlen(sheight)) == 0 || sheight[0] == 45 /* '-' */ || !png_check_fp_string(sheight, lengthh)) png_error(png_ptr, "Invalid sCAL height"); diff --git a/pngvalid.c b/pngvalid.c index d1f1fe27a..cdb612caa 100644 --- a/pngvalid.c +++ b/pngvalid.c @@ -1883,7 +1883,7 @@ modifier_read_imp(png_modifier *pm, png_bytep pb, png_size_t st) store_read_imp(&pm->this, pb, cb); pb += cb; st -= cb; - if (st <= 0) return; + if (st == 0) return; } /* No more bytes to flush, read a header, or handle a pending @@ -2325,6 +2325,8 @@ bit_size(png_structp pp, png_byte colour_type, png_byte bit_depth) { switch (colour_type) { + default: png_error(pp, "invalid color type"); + case 0: return bit_depth; case 2: return 3*bit_depth; @@ -2334,8 +2336,6 @@ bit_size(png_structp pp, png_byte colour_type, png_byte bit_depth) case 4: return 2*bit_depth; case 6: return 4*bit_depth; - - default: png_error(pp, "invalid color type"); } }