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>

2 files changed, 18 insertions, 20 deletions

diff --git a/png.c b/png.c
index cb6708f3a..c99076a03 100644
--- a/png.c
+++ b/png.c
@@ -776,6 +776,8 @@ png_access_version_number(void)
 
 /* Added at libpng version 1.2.34 and 1.4.0 (moved from pngset.c) */
 #  ifdef PNG_CHECK_cHRM_SUPPORTED
+static void png_64bit_product(long v1, long v2, long *hi_product,
+    unsigned long *lo_product);
 
 int /* PRIVATE */
 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)
 {
    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");
 
@@ -2169,29 +2172,31 @@ png_reciprocal2(png_fixed_point a, png_fixed_point b)
  *    A and D, and X || Y is (X << 16) + Y.
 */
 
-void /* PRIVATE */
-png_64bit_product (long v1, long v2, unsigned long *hi_product,
+static void
+png_64bit_product(long v1, long v2, long *hi_product,
     unsigned long *lo_product)
 {
-   int a, b, c, d;
-   long lo, hi, x, y;
+   long a, b, c, d;
+   unsigned long lo;
+   long hi, x, y;
 
-   a = (v1 >> 16) & 0xffff;
+   a = v1 >> 16;
    b = v1 & 0xffff;
-   c = (v2 >> 16) & 0xffff;
+   c = v2 >> 16;
    d = v2 & 0xffff;
 
-   lo = b * d;                   /* BD */
+   lo = b;
+   lo *= d;                      /* BD */
    x = a * d + c * b;            /* AD + CB */
-   y = ((lo >> 16) & 0xffff) + x;
+   y = (lo >> 16) + x;
 
-   lo = (lo & 0xffff) | ((y & 0xffff) << 16);
-   hi = (y >> 16) & 0xffff;
+   lo = (lo & 0xffff) | ((y & 0xffffU) << 16);
 
+   hi = y >> 16;
    hi += a * c;                  /* AC */
 
-   *hi_product = (unsigned long)hi;
-   *lo_product = (unsigned long)lo;
+   *hi_product = hi;
+   *lo_product = lo;
 }
 #endif /* CHECK_cHRM */
 
diff --git a/pngpriv.h b/pngpriv.h
index 8bdccccaf..e6ca9bf0d 100644
--- a/pngpriv.h
+++ b/pngpriv.h
@@ -1362,13 +1362,6 @@ PNG_EXTERN int png_check_cHRM_fixed PNGARG((png_structp png_ptr,
     png_fixed_point int_blue_y));
 #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
 /* Added at libpng version 1.5.5 */
 typedef struct png_xy