#574 Fix for possible OOME in Exif metadata.

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

@@ -30,15 +30,8 @@
 
 package com.twelvemonkeys.imageio.metadata.tiff;
 
-import com.twelvemonkeys.imageio.metadata.Directory;
-import com.twelvemonkeys.imageio.metadata.Entry;
-import com.twelvemonkeys.imageio.metadata.MetadataReader;
-import com.twelvemonkeys.lang.StringUtil;
-import com.twelvemonkeys.lang.Validate;
+import static com.twelvemonkeys.imageio.metadata.tiff.TIFFEntry.getValueLength;
 
-import javax.imageio.IIOException;
-import javax.imageio.ImageIO;
-import javax.imageio.stream.ImageInputStream;
 import java.io.EOFException;
 import java.io.File;
 import java.io.IOException;
@@ -46,7 +39,15 @@ import java.nio.ByteOrder;
 import java.nio.charset.StandardCharsets;
 import java.util.*;
 
-import static com.twelvemonkeys.imageio.metadata.tiff.TIFFEntry.getValueLength;
+import javax.imageio.IIOException;
+import javax.imageio.ImageIO;
+import javax.imageio.stream.ImageInputStream;
+
+import com.twelvemonkeys.imageio.metadata.Directory;
+import com.twelvemonkeys.imageio.metadata.Entry;
+import com.twelvemonkeys.imageio.metadata.MetadataReader;
+import com.twelvemonkeys.lang.StringUtil;
+import com.twelvemonkeys.lang.Validate;
 
 /**
  * TIFFReader
@@ -336,7 +337,8 @@ public final class TIFFReader extends MetadataReader {
         else {
             long valueOffset = readOffset(pInput); // This is the *value* iff the value size is <= offsetSize
 
-            if (length > 0 && length < valueOffset + valueLength) {
+            // Note: This a precaution
+            if (count >= Integer.MAX_VALUE || 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 {
@@ -367,7 +369,7 @@ public final class TIFFReader extends MetadataReader {
         long pos = pInput.getStreamPosition();
         try {
             pInput.seek(pOffset);
-            return readValue(pInput, pType, pCount);
+            return readValue(pInput, pType, pCount, longOffsets);
         }
         catch (EOFException e) {
             // TODO: Add warning listener API and report problem to client code
@@ -379,10 +381,10 @@ public final class TIFFReader extends MetadataReader {
     }
 
     private Object readValueInLine(final ImageInputStream pInput, final short pType, final int pCount) throws IOException {
-        return readValue(pInput, pType, pCount);
+        return readValue(pInput, pType, pCount, longOffsets);
     }
 
-    private static Object readValue(final ImageInputStream pInput, final short pType, final int pCount) throws IOException {
+    private static Object readValue(final ImageInputStream pInput, final short pType, final int pCount, boolean bigTIFF) throws IOException {
         // TODO: Review value "widening" for the unsigned types. Right now it's inconsistent. Should we leave it to client code?
         // TODO: New strategy: Leave data as is, instead perform the widening in TIFFEntry.getValue.
         // TODO: Add getValueByte/getValueUnsignedByte/getValueShort/getValueUnsignedShort/getValueInt/etc... in API.
@@ -513,24 +515,24 @@ public final class TIFFReader extends MetadataReader {
             case TIFF.TYPE_LONG8:
             case TIFF.TYPE_SLONG8:
             case TIFF.TYPE_IFD8:
-                // TODO: Assert BigTiff (version == 43)
+                if (bigTIFF) {
+                    if (pCount == 1) {
+                        long val = pInput.readLong();
+                        if (pType != TIFF.TYPE_SLONG8 && val < 0) {
+                            throw new IIOException(String.format("Value > %s", Long.MAX_VALUE));
+                        }
 
-                if (pCount == 1) {
-                    long val = pInput.readLong();
-                    if (pType != TIFF.TYPE_SLONG8 && val < 0) {
-                        throw new IIOException(String.format("Value > %s", Long.MAX_VALUE));
+                        return val;
                     }
 
-                    return val;
-                }
+                    long[] longs = new long[pCount];
+                    for (int i = 0; i < pCount; i++) {
+                        longs[i] = pInput.readLong();
+                    }
 
-                long[] longs = new long[pCount];
-                for (int i = 0; i < pCount; i++) {
-                    longs[i] = pInput.readLong();
+                    return longs;
                 }
 
-                return longs;
-
             default:
                 // Spec says skip unknown values
                 return new Unknown(pType, pCount, pos);

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

@@ -30,6 +30,20 @@
 
 package com.twelvemonkeys.imageio.metadata.tiff;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.*;
+
+import java.io.EOFException;
+import java.io.IOException;
+import java.io.InputStream;
+
+import javax.imageio.ImageIO;
+import javax.imageio.stream.ImageInputStream;
+import javax.imageio.stream.MemoryCacheImageInputStream;
+
+import org.junit.Test;
+
 import com.twelvemonkeys.imageio.metadata.CompoundDirectory;
 import com.twelvemonkeys.imageio.metadata.Directory;
 import com.twelvemonkeys.imageio.metadata.Entry;
@@ -38,18 +52,6 @@ import com.twelvemonkeys.imageio.metadata.exif.EXIF;
 import com.twelvemonkeys.imageio.stream.ByteArrayImageInputStream;
 import com.twelvemonkeys.imageio.stream.SubImageInputStream;
 
-import org.junit.Test;
-
-import javax.imageio.ImageIO;
-import javax.imageio.stream.ImageInputStream;
-import java.io.EOFException;
-import java.io.IOException;
-import java.io.InputStream;
-
-import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.*;
-
 /**
  * TIFFReaderTest
  *
@@ -348,6 +350,21 @@ public class TIFFReaderTest extends MetadataReaderAbstractTest {
         }
     }
 
+    @Test
+    public void testReadWithoutOOME() throws IOException {
+        // This EXIF segment from a JPEG contains an Interop IFD, containing a weird value that could be interpreted
+        // as a huge SLONG8 field (valid for BigTIFF only).
+        // OutOfMemoryError would only happen if length of stream is not known (ie. reading from underlying stream).
+        try (InputStream stream = getResource("/exif/exif-bad-interop-oome.bin").openStream()) {
+            CompoundDirectory directory = (CompoundDirectory) createReader().read(new MemoryCacheImageInputStream(stream));
+            assertEquals(2, directory.directoryCount());
+            assertEquals(11, directory.getDirectory(0).size());
+            assertEquals(6, directory.getDirectory(1).size());
+            assertEquals("Picasa", directory.getDirectory(0).getEntryById(TIFF.TAG_SOFTWARE).getValue());
+            assertEquals("2020:11:17 16:05:37", directory.getDirectory(0).getEntryById(TIFF.TAG_DATE_TIME).getValueAsString());
+        }
+    }
+
     @Test(timeout = 200)
     public void testReadCyclicExifWithoutLoopOrOOME() throws IOException {
         // This EXIF segment has an interesting bug...