#469 Fix for OOME while reading Exif with corrupted IFDs/counts

imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java

@@ -43,7 +43,7 @@ import java.io.EOFException;
 import java.io.File;
 import java.io.IOException;
 import java.nio.ByteOrder;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 
 import static com.twelvemonkeys.imageio.metadata.tiff.TIFFEntry.getValueLength;
@@ -57,9 +57,28 @@ import static com.twelvemonkeys.imageio.metadata.tiff.TIFFEntry.getValueLength;
  */
 public final class TIFFReader extends MetadataReader {
 
-    final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.metadata.exif.debug"));
+    final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.metadata.tiff.debug"));
 
-    static final Collection<Integer> KNOWN_IFDS = Collections.unmodifiableCollection(Arrays.asList(TIFF.TAG_EXIF_IFD, TIFF.TAG_GPS_IFD, TIFF.TAG_INTEROP_IFD, TIFF.TAG_SUB_IFD));
+    // TODO: Consider leaving to client code what sub-IFDs to parse (but always parse TAG_SUB_IFD).
+    private static final Collection<Integer> VALID_TOP_LEVEL_IFDS = Collections.unmodifiableCollection(Arrays.asList(TIFF.TAG_SUB_IFD, TIFF.TAG_EXIF_IFD, TIFF.TAG_GPS_IFD));
+    private static final Map<Integer, Collection<Integer>> VALID_SUB_IFDS = createSubIFDMap();
+
+    private static Map<Integer, Collection<Integer>> createSubIFDMap() {
+        HashMap<Integer, Collection<Integer>> map = new HashMap<Integer, Collection<Integer>>() {
+            @Override
+            public Collection<Integer> get(Object key) {
+                Collection<Integer> collection = super.get(key);
+                return collection != null ? collection : Collections.<Integer>emptySet();
+            }
+        };
+
+        map.put(TIFF.TAG_SUB_IFD, Collections.unmodifiableCollection(Collections.singleton(TIFF.TAG_SUB_IFD)));
+        map.put(TIFF.TAG_EXIF_IFD, Collections.unmodifiableCollection(Collections.singleton(TIFF.TAG_INTEROP_IFD)));
+
+        return Collections.unmodifiableMap(map);
+    }
+
+    private long length;
     private boolean longOffsets;
     private int offsetSize;
 
@@ -92,9 +111,9 @@ public final class TIFFReader extends MetadataReader {
             offsetSize = 8;
 
             // Just validate we're ok
-            int offsetSize = input.readUnsignedShort();
+            int offSize = input.readUnsignedShort();
-            if (offsetSize != 8) {
+            if (offSize != 8) {
-                throw new IIOException(String.format("Unexpected BigTIFF offset size: %04x, expected: %04x", offsetSize, 8));
+                throw new IIOException(String.format("Unexpected BigTIFF offset size: %04x, expected: %04x", offSize, 8));
             }
 
             int padding = input.readUnsignedShort();
@@ -106,18 +125,26 @@ public final class TIFFReader extends MetadataReader {
             throw new IIOException(String.format("Wrong TIFF magic in input data: %04x, expected: %04x", magic, TIFF.TIFF_MAGIC));
         }
 
+        length = getTIFFLength(input);
+
         return readLinkedIFDs(input);
     }
 
+    private long getTIFFLength(final ImageInputStream input) throws IOException {
+        // TODO: Scan to end, if length is unknown?
+        //  Set to fixed size? 4 GB for TIFF, BigTIFF may be huge...
+        return input.length();
+    }
+
     private TIFFDirectory readLinkedIFDs(final ImageInputStream input) throws IOException {
         long nextOffset = readOffset(input);
 
         List<IFD> ifds = new ArrayList<>();
 
         // Read linked IFDs
-        while (nextOffset != 0) {
+        while (nextOffset != 0 && (length < 0 || length > nextOffset)) {
             try {
-                ifds.add(readIFD(input, nextOffset));
+                ifds.add(readIFD(input, nextOffset, VALID_TOP_LEVEL_IFDS));
 
                 nextOffset = readOffset(input);
             }
@@ -134,7 +161,7 @@ public final class TIFFReader extends MetadataReader {
         return longOffsets ? input.readLong() : input.readUnsignedInt();
     }
 
-    private IFD readIFD(final ImageInputStream pInput, final long pOffset) throws IOException {
+    private IFD readIFD(final ImageInputStream pInput, final long pOffset, Collection<Integer> subIFDIds) throws IOException {
         pInput.seek(pOffset);
 
         long entryCount = readEntryCount(pInput);
@@ -150,14 +177,16 @@ public final class TIFFReader extends MetadataReader {
                 }
             }
             catch (IIOException e) {
+                if (DEBUG) {
+                    e.printStackTrace();
+                }
+                // TODO: Warning listener!
+
                 break;
             }
         }
 
-        // TODO: Consider leaving to client code what sub-IFDs to parse (but always parse TAG_SUB_IFD).
+        readSubIFDs(pInput, entries, subIFDIds);
-        readSubIFDs(pInput, entries,
-                Arrays.asList(TIFF.TAG_EXIF_IFD, TIFF.TAG_GPS_IFD, TIFF.TAG_INTEROP_IFD, TIFF.TAG_SUB_IFD)
-        );
 
         return new IFD(entries);
     }
@@ -166,7 +195,7 @@ public final class TIFFReader extends MetadataReader {
         return longOffsets ? pInput.readLong() : pInput.readUnsignedShort();
     }
 
-    private void readSubIFDs(ImageInputStream input, List<TIFFEntry> entries, List<Integer> subIFDIds) throws IOException {
+    private void readSubIFDs(ImageInputStream input, List<TIFFEntry> entries, Collection<Integer> subIFDIds) throws IOException {
         if (subIFDIds == null || subIFDIds.isEmpty()) {
             return;
         }
@@ -179,30 +208,28 @@ public final class TIFFReader extends MetadataReader {
 
             if (subIFDIds.contains(tagId)) {
                 try {
-                    if (KNOWN_IFDS.contains(tagId)) {
+                    long[] pointerOffsets = getPointerOffsets(entry);
-                        long[] pointerOffsets = getPointerOffsets(entry);
+                    List<IFD> subIFDs = new ArrayList<>(pointerOffsets.length);
-                        List<IFD> subIFDs = new ArrayList<>(pointerOffsets.length);
 
-                        for (long pointerOffset : pointerOffsets) {
+                    for (long pointerOffset : pointerOffsets) {
-                            try {
+                        try {
-                                subIFDs.add(readIFD(input, pointerOffset));
+                            subIFDs.add(readIFD(input, pointerOffset, VALID_SUB_IFDS.get(tagId)));
-                            }
+                        }
-                            catch (EOFException ignore) {
+                        catch (EOFException eof) {
-                                // TODO: Issue warning
+                            // TODO: Issue warning
-                                if (DEBUG) {
+                            if (DEBUG) {
-                                    ignore.printStackTrace();
+                                eof.printStackTrace();
-                                }
                             }
                         }
+                    }
 
-                        if (subIFDs.size() == 1) {
+                    if (subIFDs.size() == 1) {
-                            // Replace the entry with parsed data
+                        // Replace the entry with parsed data
-                            entries.set(i, new TIFFEntry(tagId, entry.getType(), subIFDs.get(0)));
+                        entries.set(i, new TIFFEntry(tagId, entry.getType(), subIFDs.get(0)));
-                        }
+                    }
-                        else if (!subIFDs.isEmpty()) {
+                    else if (!subIFDs.isEmpty()) {
-                            // Replace the entry with parsed data
+                        // Replace the entry with parsed data
-                            entries.set(i, new TIFFEntry(tagId, entry.getType(), subIFDs.toArray(new IFD[subIFDs.size()])));
+                        entries.set(i, new TIFFEntry(tagId, entry.getType(), subIFDs.toArray(new IFD[0])));
-                        }
                     }
                 }
                 catch (IIOException e) {
@@ -250,6 +277,7 @@ public final class TIFFReader extends MetadataReader {
         short type = pInput.readShort();
         int count = readValueCount(pInput); // Number of values
 
+        // TODO: Move this check into readValueCount?
         // It's probably a spec violation to have count 0, but we'll be lenient about it
         if (count < 0) {
             throw new IIOException(String.format("Illegal count %d for tag %s type %s @%08x", count, tagId, type, pInput.getStreamPosition()));
@@ -258,31 +286,31 @@ public final class TIFFReader extends MetadataReader {
         if (!isValidType(type)) {
             pInput.skipBytes(4); // read Value
 
-            // Invalid tag, this is just for debugging
-            long offset = pInput.getStreamPosition() - 12L;
-
             if (DEBUG) {
-                System.err.printf("Bad EXIF data @%08x\n", pInput.getStreamPosition());
+                // Invalid tag, this is just for debugging
+                long offset = pInput.getStreamPosition() - 12L;
+
+                System.err.printf("Bad TIFF data @%08x\n", pInput.getStreamPosition());
                 System.err.println("tagId: " + tagId + (tagId <= 0 ? " (INVALID)" : ""));
                 System.err.println("type: " + type + " (INVALID)");
                 System.err.println("count: " + count);
 
                 pInput.mark();
-                pInput.seek(offset);
 
                 try {
+                    pInput.seek(offset);
+
                     byte[] bytes = new byte[8 + Math.min(120, Math.max(24, count))];
                     int len = pInput.read(bytes);
 
-                    if (DEBUG) {
+                    System.err.print(HexDump.dump(offset, bytes, 0, len));
-                        System.err.print(HexDump.dump(offset, bytes, 0, len));
+                    System.err.println(len < count ? "[...]" : "");
-                        System.err.println(len < count ? "[...]" : "");
-                    }
                 }
                 finally {
                     pInput.reset();
                 }
             }
+
             return null;
         }
 
@@ -294,8 +322,14 @@ public final class TIFFReader extends MetadataReader {
             pInput.skipBytes(offsetSize - valueLength);
         }
         else {
-            long valueOffset = readOffset(pInput); // This is the *value* iff the value size is <= 4 bytes
+            long valueOffset = readOffset(pInput); // This is the *value* iff the value size is <= offsetSize
-            value = readValueAt(pInput, valueOffset, type, count);
+
+            if (length > 0 && length < valueOffset + valueLength) {
+                value = new EOFException(String.format("TIFF value offset or size too large: %d/%d bytes (length: %d bytes)", valueOffset, valueLength, length));
+            }
+            else {
+                value = readValueAt(pInput, valueOffset, type, count);
+            }
         }
 
         return new TIFFEntry(tagId, type, value);
@@ -485,7 +519,7 @@ public final class TIFFReader extends MetadataReader {
         }
     }
 
-    private static Rational createSafeRational(final long numerator, final long denominator) throws IOException {
+    private static Rational createSafeRational(final long numerator, final long denominator) {
         if (denominator == 0) {
             // Bad data.
             return Rational.NaN;
@@ -495,46 +529,29 @@ public final class TIFFReader extends MetadataReader {
     }
 
     public static void main(String[] args) throws IOException {
-//        if (true) {
-//            ImageInputStream stream = ImageIO.createImageInputStream(new File(args[0]));
-//
-//            byte[] b = new byte[Math.min((int) stream.length(), 1024)];
-//            stream.readFully(b);
-//
-//            System.err.println(HexDump.dump(b));
-//
-//            return;
-//        }
-//
         TIFFReader reader = new TIFFReader();
-        ImageInputStream stream = ImageIO.createImageInputStream(new File(args[0]));
 
-        long pos = 0;
+        try (ImageInputStream stream = ImageIO.createImageInputStream(new File(args[0]))) {
-        if (args.length > 1) {
+            long pos = 0;
-            if (args[1].startsWith("0x")) {
-                pos = Integer.parseInt(args[1].substring(2), 16);
-            }
-            else {
-                pos = Long.parseLong(args[1]);
-            }
-
-            stream.setByteOrder(pos < 0 ? ByteOrder.LITTLE_ENDIAN : ByteOrder.BIG_ENDIAN);
-
-            pos = Math.abs(pos);
-
-            stream.seek(pos);
-        }
-
-        try {
-            Directory directory;
 
             if (args.length > 1) {
-                directory = reader.readIFD(stream, pos);
+                if (args[1].startsWith("0x")) {
-            }
+                    pos = Integer.parseInt(args[1].substring(2), 16);
-            else {
+                }
-                directory = reader.read(stream);
+                else {
+                    pos = Long.parseLong(args[1]);
+                }
+
+                stream.setByteOrder(pos < 0 ? ByteOrder.LITTLE_ENDIAN : ByteOrder.BIG_ENDIAN);
+                pos = Math.abs(pos);
+
+                stream.seek(pos);
             }
 
+            Directory directory = args.length > 1
+                    ? reader.readIFD(stream, pos, VALID_TOP_LEVEL_IFDS)
+                    : reader.read(stream);
+
             for (Entry entry : directory) {
                 System.err.println(entry);
 
@@ -545,9 +562,6 @@ public final class TIFFReader extends MetadataReader {
                 }
             }
         }
-        finally {
-            stream.close();
-        }
     }
 
     //////////////////////
@@ -613,7 +627,7 @@ public final class TIFFReader extends MetadataReader {
                 }
             }
 
-            return new String(range, Charset.forName("ascii"));
+            return new String(range, StandardCharsets.US_ASCII);
         }
     }
 }

imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java

@@ -322,4 +322,26 @@ public class TIFFReaderTest extends MetadataReaderAbstractTest {
             assertEquals(15, directory.size());
         }
     }
+
+    @Test
+    public void testReadNestedExifWithoutOOME() throws IOException {
+        try (ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-with-nested-exif.jpg"))) {
+            stream.seek(30);
+            CompoundDirectory directory = (CompoundDirectory) createReader().read(new SubImageInputStream(stream, 886));
+            assertEquals(1, directory.directoryCount());
+            assertEquals(10, directory.size());
+        }
+    }
+
+    @Test
+    public void testReadExifBogusCountWithoutOOME() throws IOException {
+        try (ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-oome-bogus-count.jpg"))) {
+            stream.seek(30);
+            CompoundDirectory directory = (CompoundDirectory) createReader().read(new SubImageInputStream(stream, 3503));
+            assertEquals(2, directory.directoryCount());
+            assertEquals(12, directory.size());
+            assertEquals(9, directory.getDirectory(0).size());
+            assertEquals(3, directory.getDirectory(1).size());
+        }
+    }
 }