8358057: Update validation of ICC_Profile header data

mrserb · mrserb · commit 8939acc8ab0e · 2025-06-04T17:53:17.000Z
Reviewed-by: honkar
diff --git a/src/java.desktop/share/classes/java/awt/color/ICC_Profile.java b/src/java.desktop/share/classes/java/awt/color/ICC_Profile.java
@@ -811,12 +811,12 @@ public static ICC_Profile getInstance(byte[] data) {
         }
 
         try {
-            if (getColorSpaceType(p) == ColorSpace.TYPE_GRAY
+            if (getColorSpaceType(data) == ColorSpace.TYPE_GRAY
                     && getData(p, icSigMediaWhitePointTag) != null
                     && getData(p, icSigGrayTRCTag) != null) {
                 return new ICC_ProfileGray(p);
             }
-            if (getColorSpaceType(p) == ColorSpace.TYPE_RGB
+            if (getColorSpaceType(data) == ColorSpace.TYPE_RGB
                     && getData(p, icSigMediaWhitePointTag) != null
                     && getData(p, icSigRedColorantTag) != null
                     && getData(p, icSigGreenColorantTag) != null
@@ -1028,13 +1028,8 @@ public int getColorSpaceType() {
         if (info != null) {
             return info.colorSpaceType;
         }
-        return getColorSpaceType(cmmProfile());
-    }
-
-    private static int getColorSpaceType(Profile p) {
-        byte[] theHeader = getData(p, icSigHead);
-        int theColorSpaceSig = intFromBigEndian(theHeader, icHdrColorSpace);
-        return iccCStoJCS(theColorSpaceSig);
+        byte[] theHeader = getData(cmmProfile(), icSigHead);
+        return getColorSpaceType(theHeader);
     }
 
     private static int getColorSpaceType(byte[] theHeader) {
@@ -1057,8 +1052,7 @@ private static int getColorSpaceType(byte[] theHeader) {
      */
     public int getPCSType() {
         byte[] theHeader = getData(icSigHead);
-        int thePCSSig = intFromBigEndian(theHeader, icHdrPcs);
-        return iccCStoJCS(thePCSSig);
+        return getPCSType(theHeader);
     }
 
     private static int getPCSType(byte[] theHeader) {
@@ -1189,23 +1183,19 @@ private static void verifyHeader(byte[] data) {
         checkRenderingIntent(data);
     }
 
-    private static boolean checkRenderingIntent(byte[] header) {
+    private static void checkRenderingIntent(byte[] header) {
         int index = ICC_Profile.icHdrRenderingIntent;
-
-        /* According to ICC spec, only the least-significant 16 bits shall be
-         * used to encode the rendering intent. The most significant 16 bits
-         * shall be set to zero. Thus, we are ignoring two most significant
-         * bytes here. Please refer ICC Spec Document for more details.
+        /*
+         * ICC spec: only the least-significant 16 bits encode the rendering
+         * intent. The most significant 16 bits must be zero and can be ignored.
+         * https://www.color.org/specification/ICC.1-2022-05.pdf, section 7.2.15
          */
-        int renderingIntent = ((header[index+2] & 0xff) <<  8) |
-                              (header[index+3] & 0xff);
-
-        switch (renderingIntent) {
-            case icPerceptual, icMediaRelativeColorimetric,
-                    icSaturation, icAbsoluteColorimetric -> {
-                return true;
-            }
-            default -> throw new IllegalArgumentException("Unknown Rendering Intent");
+        // Extract 16-bit unsigned rendering intent (0–65535)
+        int intent = (header[index + 2] & 0xff) << 8 | header[index + 3] & 0xff;
+        // Only check upper bound since intent can't be negative
+        if (intent > icICCAbsoluteColorimetric) {
+            throw new IllegalArgumentException(
+                    "Unknown Rendering Intent: %d".formatted(intent));
         }
     }
 
diff --git a/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java b/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -684,13 +684,10 @@ public final Point2D getPoint2D (Point2D srcPt, Point2D dstPt) {
     private int getRenderingIntent (ICC_Profile profile) {
         byte[] header = profile.getData(ICC_Profile.icSigHead);
         int index = ICC_Profile.icHdrRenderingIntent;
-
-        /* According to ICC spec, only the least-significant 16 bits shall be
-         * used to encode the rendering intent. The most significant 16 bits
-         * shall be set to zero. Thus, we are ignoring two most significant
-         * bytes here.
-         *
-         *  See https://www.color.org/ICC1v42_2006-05.pdf, section 7.2.15.
+        /*
+         * ICC spec: only the least-significant 16 bits encode the rendering
+         * intent. The most significant 16 bits must be zero and can be ignored.
+         * https://www.color.org/specification/ICC.1-2022-05.pdf, section 7.2.15
          */
         return ((header[index+2] & 0xff) <<  8) |
                 (header[index+3] & 0xff);
diff --git a/test/jdk/java/awt/color/ICC_Profile/RenderingIntentStressTest.java b/test/jdk/java/awt/color/ICC_Profile/RenderingIntentStressTest.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.awt.color.ColorSpace;
+import java.awt.color.ICC_Profile;
+
+import static java.awt.color.ICC_Profile.icAbsoluteColorimetric;
+import static java.awt.color.ICC_Profile.icICCAbsoluteColorimetric;
+import static java.awt.color.ICC_Profile.icMediaRelativeColorimetric;
+import static java.awt.color.ICC_Profile.icPerceptual;
+import static java.awt.color.ICC_Profile.icRelativeColorimetric;
+import static java.awt.color.ICC_Profile.icSaturation;
+
+/**
+ * @test
+ * @bug 8358057
+ * @summary Stress test for ICC_Profile rendering intent parsing and validation
+ */
+public final class RenderingIntentStressTest {
+
+    public static void main(String[] args) {
+        ICC_Profile builtin = ICC_Profile.getInstance(ColorSpace.CS_sRGB);
+        ICC_Profile profile = ICC_Profile.getInstance(builtin.getData());
+        // some random combinations that should be ignored
+        int[] upperBytes = {0x0000, 0xFFFF, 0xA5A5, 0x8000, 0x0001, 0x8080,
+                            0x0101, 0xAA55, 0x550A, 0xFF00};
+        for (int up : upperBytes) {
+            for (int low = 0; low <= 0xFFFF; low++) {
+                test(profile, up, low);
+            }
+        }
+    }
+
+    private static int getRenderingIntent(byte[] header) {
+        // replicate the logic we have in jdk
+        int index = ICC_Profile.icHdrRenderingIntent;
+        return (header[index + 2] & 0xff) << 8 | header[index + 3] & 0xff;
+    }
+
+    private static void test(ICC_Profile profile, int up, int low) {
+        byte[] header = profile.getData(ICC_Profile.icSigHead);
+        // These bytes should be ignored
+        header[ICC_Profile.icHdrRenderingIntent + 0] = (byte) (up >> 8 & 0xFF);
+        header[ICC_Profile.icHdrRenderingIntent + 1] = (byte) (up & 0xFF);
+        // This is the actual intent
+        header[ICC_Profile.icHdrRenderingIntent + 2] = (byte) (low >> 8 & 0xFF);
+        header[ICC_Profile.icHdrRenderingIntent + 3] = (byte) (low & 0xFF);
+
+        boolean isValid = isValidIntent(low);
+        try {
+            profile.setData(ICC_Profile.icSigHead, header);
+            if (!isValid) {
+                throw new RuntimeException("IAE is expected");
+            }
+        } catch (IllegalArgumentException e) {
+            if (isValid) {
+                throw e;
+            }
+            return;
+        }
+        // verify that the intent is correctly stored in the profile by the CMM
+        byte[] data = profile.getData(ICC_Profile.icSigHead);
+        int actualIntent = getRenderingIntent(data);
+        if (actualIntent != low) {
+            System.out.println("Expected: " + low);
+            System.out.println("Actual: " + actualIntent);
+            throw new RuntimeException("Unexpected intent");
+        }
+    }
+
+    private static boolean isValidIntent(int intent) {
+        return intent == icPerceptual || intent == icRelativeColorimetric
+                || intent == icMediaRelativeColorimetric
+                || intent == icSaturation || intent == icAbsoluteColorimetric
+                || intent == icICCAbsoluteColorimetric;
+    }
+}
diff --git a/test/jdk/java/awt/color/ICC_Profile/ValidateICCHeaderData/ValidateICCHeaderData.java b/test/jdk/java/awt/color/ICC_Profile/ValidateICCHeaderData/ValidateICCHeaderData.java
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8337703
+ * @bug 8347377 8358057
  * @summary To verify if ICC_Profile's setData() and getInstance() methods
  *          validate header data and throw IAE for invalid values.
  * @run main ValidateICCHeaderData
@@ -144,9 +144,7 @@ public static void main(String[] args) throws Exception {
         System.out.println("CASE 10: Passed \n");
 
         System.out.println("CASE 11: Testing INVALID Rendering Intent ...");
-        //valid rendering intent values are 0-3
-        int invalidRenderIntent = 5;
-        testInvalidHeaderData(invalidRenderIntent, RENDER_INTENT_START_INDEX, 4);
+        testInvalidIntent();
         System.out.println("CASE 11: Passed \n");
 
         System.out.println("CASE 12: Testing INVALID Header Size ...");
@@ -187,6 +185,21 @@ private static void testInvalidHeaderData(int invalidData, int startIndex,
         }
     }
 
+    private static void testInvalidIntent() {
+        //valid rendering intent values are 0-3
+        int invalidRenderIntent = 5;
+        try {
+            setTag(invalidRenderIntent, RENDER_INTENT_START_INDEX, 4);
+            throw new RuntimeException("Test Failed ! Expected IAE NOT thrown");
+        } catch (IllegalArgumentException iae) {
+            String message = iae.getMessage();
+            System.out.println("Expected IAE thrown: " + message);
+            if (!message.contains(": " + invalidRenderIntent)) {
+                throw new RuntimeException("Test Failed ! Unexpected text");
+            }
+        }
+    }
+
     private static void setTag(int value, int startIndex, int fieldLength) {
         byte[] byteArray;
         if (startIndex == RENDER_INTENT_START_INDEX) {
