Eliminate signed overlow in png_64bit_product

The previous version produced a signed overflow as a result of both the
& 0xffff on the most significant bits of a negative argument; this
converted (-1) into 65535 which resulted in a subsequent overflow.
Since signed overflow is undefined in C90 the code has been modified to
correctly calculate a signed result.  This requires changing the 'hi'
result parametr to a signed value.

This has been code reviewed solely by the author.  A further code review
is highly desireable.  Nevertheless the code compiles without warnings
from clang and without the prior detection of an overflow.  Since it no
longer truncates any of the intermediate values this should be enough to
ensure that it is correct.

Signed-off-by: John Bowler <jbowler@acm.org>
This commit is contained in:
John Bowler 2016-12-26 16:29:16 -08:00
parent 09dcb906a7
commit 95e0094f85
2 changed files with 18 additions and 20 deletions

31
png.c
View File

@ -776,6 +776,8 @@ png_access_version_number(void)
/* Added at libpng version 1.2.34 and 1.4.0 (moved from pngset.c) */ /* Added at libpng version 1.2.34 and 1.4.0 (moved from pngset.c) */
# ifdef PNG_CHECK_cHRM_SUPPORTED # ifdef PNG_CHECK_cHRM_SUPPORTED
static void png_64bit_product(long v1, long v2, long *hi_product,
unsigned long *lo_product);
int /* PRIVATE */ int /* PRIVATE */
png_check_cHRM_fixed(png_structp png_ptr, png_check_cHRM_fixed(png_structp png_ptr,
@ -784,7 +786,8 @@ png_check_cHRM_fixed(png_structp png_ptr,
png_fixed_point blue_x, png_fixed_point blue_y) png_fixed_point blue_x, png_fixed_point blue_y)
{ {
int ret = 1; int ret = 1;
unsigned long xy_hi,xy_lo,yx_hi,yx_lo; long xy_hi,yx_hi;
unsigned long xy_lo,yx_lo;
png_debug(1, "in function png_check_cHRM_fixed"); png_debug(1, "in function png_check_cHRM_fixed");
@ -2169,29 +2172,31 @@ png_reciprocal2(png_fixed_point a, png_fixed_point b)
* A and D, and X || Y is (X << 16) + Y. * A and D, and X || Y is (X << 16) + Y.
*/ */
void /* PRIVATE */ static void
png_64bit_product (long v1, long v2, unsigned long *hi_product, png_64bit_product(long v1, long v2, long *hi_product,
unsigned long *lo_product) unsigned long *lo_product)
{ {
int a, b, c, d; long a, b, c, d;
long lo, hi, x, y; unsigned long lo;
long hi, x, y;
a = (v1 >> 16) & 0xffff; a = v1 >> 16;
b = v1 & 0xffff; b = v1 & 0xffff;
c = (v2 >> 16) & 0xffff; c = v2 >> 16;
d = v2 & 0xffff; d = v2 & 0xffff;
lo = b * d; /* BD */ lo = b;
lo *= d; /* BD */
x = a * d + c * b; /* AD + CB */ x = a * d + c * b; /* AD + CB */
y = ((lo >> 16) & 0xffff) + x; y = (lo >> 16) + x;
lo = (lo & 0xffff) | ((y & 0xffff) << 16); lo = (lo & 0xffff) | ((y & 0xffffU) << 16);
hi = (y >> 16) & 0xffff;
hi = y >> 16;
hi += a * c; /* AC */ hi += a * c; /* AC */
*hi_product = (unsigned long)hi; *hi_product = hi;
*lo_product = (unsigned long)lo; *lo_product = lo;
} }
#endif /* CHECK_cHRM */ #endif /* CHECK_cHRM */

View File

@ -1362,13 +1362,6 @@ PNG_EXTERN int png_check_cHRM_fixed PNGARG((png_structp png_ptr,
png_fixed_point int_blue_y)); png_fixed_point int_blue_y));
#endif #endif
#ifdef PNG_CHECK_cHRM_SUPPORTED
/* Added at libpng version 1.2.34 and 1.4.0 */
/* Currently only used by png_check_cHRM_fixed */
PNG_EXTERN void png_64bit_product PNGARG((long v1, long v2,
unsigned long *hi_product, unsigned long *lo_product));
#endif
#ifdef PNG_cHRM_SUPPORTED #ifdef PNG_cHRM_SUPPORTED
/* Added at libpng version 1.5.5 */ /* Added at libpng version 1.5.5 */
typedef struct png_xy typedef struct png_xy