fix: Prevent overflow in chromaticity calculations

In `png_xy_from_XYZ` X+Y+Z was calculated without checking for overflow.
This fixes that by moving the correct code from `png_XYZ_normalize` into
a static function which is now used from `png_xy_from_XYZ`.

Reviewed-by: Cosmin Truta <[email protected]>
Signed-off-by: John Bowler <[email protected]>
Signed-off-by: Cosmin Truta <[email protected]>

png.c

@@ -1203,6 +1203,24 @@ png_colorspace_sync(png_const_structrp png_ptr, png_inforp info_ptr)
 #endif /* GAMMA */
 
 #ifdef PNG_COLORSPACE_SUPPORTED
+static int
+png_safe_add(png_int_32 *addend0_and_result, png_int_32 addend1,
+      png_int_32 addend2) {
+   /* Safely add three integers.  Returns 0 on success, 1 on overlow.
+    * IMPLEMENTATION NOTE: ANSI requires signed overflow not to occur, therefore
+    * relying on addition of two positive values producing a negative one is not
+    * safe.
+    */
+   int addend0 = *addend0_and_result;
+   if (0x7fffffff - addend0 < addend1)
+      return 1;
+   addend0 += addend1;
+   if (0x7fffffff - addend1 < addend2)
+      return 1;
+   *addend0_and_result = addend0 + addend2;
+   return 0;
+}
+
 /* Added at libpng-1.5.5 to support read and write of true CIEXYZ values for
  * cHRM, as opposed to using chromaticities.  These internal APIs return
  * non-zero on a parameter error.  The X, Y and Z values are required to be
@@ -1211,38 +1229,52 @@ png_colorspace_sync(png_const_structrp png_ptr, png_inforp info_ptr)
 static int
 png_xy_from_XYZ(png_xy *xy, const png_XYZ *XYZ)
 {
-   png_int_32 d, dwhite, whiteX, whiteY;
+   png_int_32 d, dred, dgreen, dwhite, whiteX, whiteY;
 
-   d = XYZ->red_X + XYZ->red_Y + XYZ->red_Z;
+   /* 'd' in each of the blocks below is just X+Y+Z for each component,
+    * x, y and z are X,Y,Z/(X+Y+Z).
+    */
+   d = XYZ->red_X;
+   if (png_safe_add(&d, XYZ->red_Y, XYZ->red_Z))
+      return 1;
    if (png_muldiv(&xy->redx, XYZ->red_X, PNG_FP_1, d) == 0)
       return 1;
    if (png_muldiv(&xy->redy, XYZ->red_Y, PNG_FP_1, d) == 0)
       return 1;
-   dwhite = d;
+   dred = d;
    whiteX = XYZ->red_X;
    whiteY = XYZ->red_Y;
 
-   d = XYZ->green_X + XYZ->green_Y + XYZ->green_Z;
+   d = XYZ->green_X;
+   if (png_safe_add(&d, XYZ->green_Y, XYZ->green_Z))
+      return 1;
    if (png_muldiv(&xy->greenx, XYZ->green_X, PNG_FP_1, d) == 0)
       return 1;
    if (png_muldiv(&xy->greeny, XYZ->green_Y, PNG_FP_1, d) == 0)
       return 1;
-   dwhite += d;
+   dgreen = d;
    whiteX += XYZ->green_X;
    whiteY += XYZ->green_Y;
 
-   d = XYZ->blue_X + XYZ->blue_Y + XYZ->blue_Z;
+   d = XYZ->blue_X;
+   if (png_safe_add(&d, XYZ->blue_Y, XYZ->blue_Z))
+      return 1;
    if (png_muldiv(&xy->bluex, XYZ->blue_X, PNG_FP_1, d) == 0)
       return 1;
    if (png_muldiv(&xy->bluey, XYZ->blue_Y, PNG_FP_1, d) == 0)
       return 1;
-   dwhite += d;
    whiteX += XYZ->blue_X;
    whiteY += XYZ->blue_Y;
 
-   /* The reference white is simply the sum of the end-point (X,Y,Z) vectors,
-    * thus:
+   /* The reference white is simply the sum of the end-point (X,Y,Z) vectors so
+    * the fillowing calculates (X+Y+Z) of the reference white (media white,
+    * encoding white) itself:
     */
+   if (png_safe_add(&d, dred, dgreen))
+      return 1;
+
+   dwhite = d;
+
    if (png_muldiv(&xy->whitex, whiteX, PNG_FP_1, dwhite) == 0)
       return 1;
    if (png_muldiv(&xy->whitey, whiteY, PNG_FP_1, dwhite) == 0)
@@ -1506,25 +1538,14 @@ png_XYZ_from_xy(png_XYZ *XYZ, const png_xy *xy)
 static int
 png_XYZ_normalize(png_XYZ *XYZ)
 {
-   png_int_32 Y;
+   png_int_32 Y, Ytemp;
 
-   if (XYZ->red_Y < 0 || XYZ->green_Y < 0 || XYZ->blue_Y < 0 ||
-      XYZ->red_X < 0 || XYZ->green_X < 0 || XYZ->blue_X < 0 ||
-      XYZ->red_Z < 0 || XYZ->green_Z < 0 || XYZ->blue_Z < 0)
+   /* Normalize by scaling so the sum of the end-point Y values is PNG_FP_1. */
+   Ytemp = XYZ->red_Y;
+   if (png_safe_add(&Ytemp, XYZ->green_Y, XYZ->blue_Y))
       return 1;
 
-   /* Normalize by scaling so the sum of the end-point Y values is PNG_FP_1.
-    * IMPLEMENTATION NOTE: ANSI requires signed overflow not to occur, therefore
-    * relying on addition of two positive values producing a negative one is not
-    * safe.
-    */
-   Y = XYZ->red_Y;
-   if (0x7fffffff - Y < XYZ->green_X)
-      return 1;
-   Y += XYZ->green_Y;
-   if (0x7fffffff - Y < XYZ->blue_X)
-      return 1;
-   Y += XYZ->blue_Y;
+   Y = Ytemp;
 
    if (Y != PNG_FP_1)
    {