diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java index 6fb038a4..fc89031f 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java +++ b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java @@ -30,6 +30,37 @@ package com.twelvemonkeys.imageio.plugins.jpeg; +import java.awt.*; +import java.awt.color.ColorSpace; +import java.awt.color.ICC_ColorSpace; +import java.awt.color.ICC_Profile; +import java.awt.image.*; +import java.io.DataInput; +import java.io.DataInputStream; +import java.io.EOFException; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.SequenceInputStream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +import javax.imageio.IIOException; +import javax.imageio.ImageIO; +import javax.imageio.ImageReadParam; +import javax.imageio.ImageReader; +import javax.imageio.ImageTypeSpecifier; +import javax.imageio.event.IIOReadUpdateListener; +import javax.imageio.event.IIOReadWarningListener; +import javax.imageio.metadata.IIOMetadata; +import javax.imageio.metadata.IIOMetadataFormatImpl; +import javax.imageio.metadata.IIOMetadataNode; +import javax.imageio.spi.ImageReaderSpi; +import javax.imageio.stream.ImageInputStream; + import com.twelvemonkeys.imageio.ImageReaderBase; import com.twelvemonkeys.imageio.color.ColorSpaces; import com.twelvemonkeys.imageio.color.YCbCrConverter; @@ -49,24 +80,6 @@ import com.twelvemonkeys.imageio.util.ProgressListenerBase; import com.twelvemonkeys.lang.Validate; import com.twelvemonkeys.xml.XMLSerializer; -import javax.imageio.*; -import javax.imageio.event.IIOReadUpdateListener; -import javax.imageio.event.IIOReadWarningListener; -import javax.imageio.metadata.IIOMetadata; -import javax.imageio.metadata.IIOMetadataFormatImpl; -import javax.imageio.metadata.IIOMetadataNode; -import javax.imageio.spi.ImageReaderSpi; -import javax.imageio.stream.ImageInputStream; -import javax.imageio.stream.MemoryCacheImageInputStream; -import java.awt.*; -import java.awt.color.ColorSpace; -import java.awt.color.ICC_ColorSpace; -import java.awt.color.ICC_Profile; -import java.awt.image.*; -import java.io.*; -import java.util.List; -import java.util.*; - /** * A JPEG {@code ImageReader} implementation based on the JRE {@code JPEGImageReader}, * that adds support and properly handles cases where the JRE version throws exceptions. @@ -896,16 +909,16 @@ public final class JPEGImageReader extends ImageReaderBase { if (!exifSegments.isEmpty()) { Application exif = exifSegments.get(0); - InputStream data = exif.data(); + int offset = exif.identifier.length() + 2; // Incl. pad - if (data.read() == -1) { // Read pad + if (exif.data.length <= offset) { processWarningOccurred("Exif chunk has no data."); } else { - ImageInputStream stream = new MemoryCacheImageInputStream(data); - return (CompoundDirectory) new TIFFReader().read(stream); - - // TODO: Directory offset of thumbnail is wrong/relative to container stream, causing trouble for the TIFFReader... + // TODO: Consider returning ByteArrayImageInputStream from Segment.data() + try (ImageInputStream stream = new ByteArrayImageInputStream(exif.data, offset, exif.data.length - offset)) { + return (CompoundDirectory) new TIFFReader().read(stream); + } } } diff --git a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java index 7dd497c2..6d2cf773 100644 --- a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java +++ b/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); diff --git a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java index 41f61335..06e38559 100644 --- a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java +++ b/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... diff --git a/imageio/imageio-metadata/src/test/resources/exif/exif-bad-interop-oome.bin b/imageio/imageio-metadata/src/test/resources/exif/exif-bad-interop-oome.bin new file mode 100644 index 00000000..accc1c69 Binary files /dev/null and b/imageio/imageio-metadata/src/test/resources/exif/exif-bad-interop-oome.bin differ