8169728: Missing sign checks in TIFFField(TIFFTag tag, int type, int count, Object data) for TIFFTag.TIFF_LONG
authorbpb
Fri, 09 Dec 2016 13:48:06 -0800
changeset 42743 e61f0c011e93
parent 42742 60484cf11911
child 42744 a2fcebac9799
8169728: Missing sign checks in TIFFField(TIFFTag tag, int type, int count, Object data) for TIFFTag.TIFF_LONG Summary: Add further checks for TIFF_LONG, TIFF_RATIONAL, TIFF_SRATIONAL, and TIFF_IFD_POINTER. Reviewed-by: prr
jdk/src/java.desktop/share/classes/javax/imageio/plugins/tiff/TIFFField.java
jdk/test/javax/imageio/plugins/tiff/TIFFDirectoryTest.java
jdk/test/javax/imageio/plugins/tiff/TIFFFieldTest.java
--- a/jdk/src/java.desktop/share/classes/javax/imageio/plugins/tiff/TIFFField.java	Fri Dec 09 11:08:38 2016 -0800
+++ b/jdk/src/java.desktop/share/classes/javax/imageio/plugins/tiff/TIFFField.java	Fri Dec 09 13:48:06 2016 -0800
@@ -263,14 +263,16 @@
  */
 public final class TIFFField implements Cloneable {
 
-    private static final String[] typeNames = {
+    private static final long MAX_UINT32 = 0xffffffffL;
+
+    private static final String[] TYPE_NAMES = {
         null,
         "Byte", "Ascii", "Short", "Long", "Rational",
         "SByte", "Undefined", "SShort", "SLong", "SRational",
         "Float", "Double", "IFDPointer"
     };
 
-    private static final boolean[] isIntegral = {
+    private static final boolean[] IS_INTEGRAL = {
         false,
         true, false, true, true, false,
         true, true, true, true, false,
@@ -544,6 +546,9 @@
      * @throws IllegalArgumentException if {@code data} is an instance of
      * a class incompatible with the specified type.
      * @throws IllegalArgumentException if the size of the data array is wrong.
+     * @throws IllegalArgumentException if the type of the data array is
+     * {@code TIFF_LONG}, {@code TIFF_RATIONAL}, or {@code TIFF_IFD_POINTER}
+     * and any of the elements is negative or greater than {@code 0xffffffff}.
      */
     public TIFFField(TIFFTag tag, int type, int count, Object data) {
         if(tag == null) {
@@ -587,15 +592,50 @@
         case TIFFTag.TIFF_LONG:
             isDataArrayCorrect = data instanceof long[]
                 && ((long[])data).length == count;
+            if (isDataArrayCorrect) {
+                for (long datum : (long[])data) {
+                    if (datum < 0) {
+                        throw new IllegalArgumentException
+                            ("Negative value supplied for TIFF_LONG");
+                    }
+                    if (datum > MAX_UINT32) {
+                        throw new IllegalArgumentException
+                            ("Too large value supplied for TIFF_LONG");
+                    }
+                }
+            }
             break;
         case TIFFTag.TIFF_IFD_POINTER:
             isDataArrayCorrect = data instanceof long[]
                 && ((long[])data).length == 1;
+            if (((long[])data)[0] < 0) {
+                throw new IllegalArgumentException
+                    ("Negative value supplied for TIFF_IFD_POINTER");
+            }
+            if (((long[])data)[0] > MAX_UINT32) {
+                throw new IllegalArgumentException
+                    ("Too large value supplied for TIFF_IFD_POINTER");
+            }
             break;
         case TIFFTag.TIFF_RATIONAL:
             isDataArrayCorrect = data instanceof long[][]
-                && ((long[][])data).length == count
-                && ((long[][])data)[0].length == 2;
+                && ((long[][])data).length == count;
+            if (isDataArrayCorrect) {
+                for (long[] datum : (long[][])data) {
+                    if (datum.length != 2) {
+                        isDataArrayCorrect = false;
+                        break;
+                    }
+                    if (datum[0] < 0 || datum[1] < 0) {
+                        throw new IllegalArgumentException
+                            ("Negative value supplied for TIFF_RATIONAL");
+                    }
+                    if (datum[0] > MAX_UINT32 || datum[1] > MAX_UINT32) {
+                        throw new IllegalArgumentException
+                            ("Too large value supplied for TIFF_RATIONAL");
+                    }
+                }
+            }
             break;
         case TIFFTag.TIFF_SSHORT:
             isDataArrayCorrect = data instanceof short[]
@@ -607,8 +647,15 @@
             break;
         case TIFFTag.TIFF_SRATIONAL:
             isDataArrayCorrect = data instanceof int[][]
-                && ((int[][])data).length == count
-                && ((int[][])data)[0].length == 2;
+                && ((int[][])data).length == count;
+            if (isDataArrayCorrect) {
+                for (int[] datum : (int[][])data) {
+                    if (datum.length != 2) {
+                        isDataArrayCorrect = false;
+                        break;
+                    }
+                }
+            }
             break;
         case TIFFTag.TIFF_FLOAT:
             isDataArrayCorrect = data instanceof float[]
@@ -681,7 +728,7 @@
         if (value < 0) {
             throw new IllegalArgumentException("value < 0!");
         }
-        if (value > 0xffffffffL) {
+        if (value > MAX_UINT32) {
             throw new IllegalArgumentException("value > 0xffffffff!");
         }
 
@@ -806,7 +853,7 @@
             throw new IllegalArgumentException("Unknown data type "+dataType);
         }
 
-        return typeNames[dataType];
+        return TYPE_NAMES[dataType];
     }
 
     /**
@@ -819,7 +866,7 @@
      */
     public static int getTypeByName(String typeName) {
         for (int i = TIFFTag.MIN_DATATYPE; i <= TIFFTag.MAX_DATATYPE; i++) {
-            if (typeName.equals(typeNames[i])) {
+            if (typeName.equals(TYPE_NAMES[i])) {
                 return i;
             }
         }
@@ -894,7 +941,7 @@
      * @return Whether the field type is integral.
      */
     public boolean isIntegral() {
-        return isIntegral[type];
+        return IS_INTEGRAL[type];
     }
 
     /**
--- a/jdk/test/javax/imageio/plugins/tiff/TIFFDirectoryTest.java	Fri Dec 09 11:08:38 2016 -0800
+++ b/jdk/test/javax/imageio/plugins/tiff/TIFFDirectoryTest.java	Fri Dec 09 13:48:06 2016 -0800
@@ -154,7 +154,7 @@
             "must return null TIFFField");
 
         long offset = 4L;
-        long a[] = {Long.MIN_VALUE, 0, Long.MAX_VALUE};
+        long a[] = {0, Integer.MAX_VALUE, (1 << 32) - 1};
         int v = 100500;
         TIFFField
                 f1 = new TIFFField(tag1, type, offset, d),
--- a/jdk/test/javax/imageio/plugins/tiff/TIFFFieldTest.java	Fri Dec 09 11:08:38 2016 -0800
+++ b/jdk/test/javax/imageio/plugins/tiff/TIFFFieldTest.java	Fri Dec 09 13:48:06 2016 -0800
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug     8152183 8149562 8169725
+ * @bug     8152183 8149562 8169725 8169728
  * @author  a.stepanov
  * @summary Some checks for TIFFField methods
  * @run     main TIFFFieldTest
@@ -171,6 +171,94 @@
         check((f.getDirectory() == null) && !f.hasDirectory(),
             "must not have directory");
 
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_RATIONAL);
+            long[][] tiffRationals = new long[6][3];
+            new TIFFField(t, TIFFTag.TIFF_RATIONAL, tiffRationals.length,
+                tiffRationals);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_SRATIONAL);
+            int[][] tiffSRationals = new int[6][3];
+            new TIFFField(t, TIFFTag.TIFF_SRATIONAL, tiffSRationals.length,
+                tiffSRationals);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_LONG);
+            long[] tiffLongs = new long[] {0, -7, 10};
+            new TIFFField(t, TIFFTag.TIFF_LONG, tiffLongs.length,
+                tiffLongs);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_LONG);
+            long[] tiffLongs = new long[] {0, 7, 0x100000000L};
+            new TIFFField(t, TIFFTag.TIFF_LONG, tiffLongs.length,
+                tiffLongs);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_IFD_POINTER);
+            long[] tiffLongs = new long[] {-7};
+            new TIFFField(t, TIFFTag.TIFF_IFD_POINTER, tiffLongs.length,
+                tiffLongs);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_IFD_POINTER);
+            long[] tiffLongs = new long[] {0x100000000L};
+            new TIFFField(t, TIFFTag.TIFF_IFD_POINTER, tiffLongs.length,
+                tiffLongs);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_RATIONAL);
+            long[][] tiffRationals = new long[][] {
+                {10, 2},
+                {1, -3},
+                {4,  7}
+            };
+            new TIFFField(t, TIFFTag.TIFF_RATIONAL, tiffRationals.length,
+                tiffRationals);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
+        ok = false;
+        try {
+            TIFFTag t = new TIFFTag(NAME, NUM, 1 << TIFFTag.TIFF_RATIONAL);
+            long[][] tiffRationals = new long[][] {
+                {10, 2},
+                {0x100000000L, 3},
+                {4,  7}
+            };
+            new TIFFField(t, TIFFTag.TIFF_RATIONAL, tiffRationals.length,
+                tiffRationals);
+        } catch (IllegalArgumentException e) {
+            ok = true;
+        }
+
         // constructor: TIFFField(tag, type, offset, dir)
         List<TIFFTag> tags = new ArrayList<>();
         tags.add(tag);