From 5298282c14f63380a075ed3061d286f69e000431 Mon Sep 17 00:00:00 2001 From: Shuyang Feng Date: Fri, 30 Oct 2026 20:45:50 +1100 Subject: [PATCH 1/2] [IMAGING-371] Preserve GPSVersionID when reading GPS directories --- .../imaging/formats/tiff/TiffReader.java | 3 +- .../formats/tiff/TiffGpsVersionIdTest.java | 103 ++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java diff --git a/src/main/java/org/apache/commons/imaging/formats/tiff/TiffReader.java b/src/main/java/org/apache/commons/imaging/formats/tiff/TiffReader.java index 8edd59df6..ff93c9daf 100644 --- a/src/main/java/org/apache/commons/imaging/formats/tiff/TiffReader.java +++ b/src/main/java/org/apache/commons/imaging/formats/tiff/TiffReader.java @@ -382,7 +382,8 @@ private boolean readDirectory(final ByteSource byteSource, final long directoryO offset = ByteConversions.toLong(offsetBytes, getByteOrder()); } - if (tag == 0) { + if (tag == 0 && dirType != TiffDirectoryConstants.DIRECTORY_TYPE_GPS) { + // Tag zero is GPSVersionID in the GPS directory. // skip invalid fields. // These are seen very rarely, but can have invalid value // lengths, diff --git a/src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java b/src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java new file mode 100644 index 000000000..1c9feba1a --- /dev/null +++ b/src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.imaging.formats.tiff; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.util.stream.Stream; + +import org.apache.commons.imaging.FormatCompliance; +import org.apache.commons.imaging.Imaging; +import org.apache.commons.imaging.bytesource.ByteSource; +import org.apache.commons.imaging.formats.tiff.constants.GpsTagConstants; +import org.apache.commons.imaging.formats.tiff.constants.TiffDirectoryConstants; +import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +/** Tests IMAGING-371 without depending on the TIFF writer or external images. */ +class TiffGpsVersionIdTest { + + private static byte[] createGpsMetadata(final ByteOrder order) { + final ByteBuffer buffer = ByteBuffer.allocate(56).order(order); + final byte marker = (byte) (order == ByteOrder.LITTLE_ENDIAN ? 'I' : 'M'); + buffer.put(marker).put(marker).putShort((short) 42).putInt(8); + buffer.putShort((short) 1); + // Root GPSInfo pointer: LONG[1], pointing to the GPS IFD at offset 26. + buffer.putShort((short) 0x8825).putShort((short) 4).putInt(1).putInt(26); + buffer.putInt(0); + buffer.putShort((short) 2); + // GPSVersionID: BYTE[4], stored inline. Tag zero is valid in this IFD. + buffer.putShort((short) 0).putShort((short) 1).putInt(4).put(new byte[] { 2, 3, 0, 0 }); + // GPSLatitudeRef is a control for directory traversal and entry alignment. + buffer.putShort((short) 1).putShort((short) 2).putInt(2).put(new byte[] { 'N', 0, 0, 0 }); + buffer.putInt(0); + return buffer.array(); + } + + private static Stream readerOptions() { + return Stream.of(Arguments.of(ByteOrder.LITTLE_ENDIAN, false), Arguments.of(ByteOrder.LITTLE_ENDIAN, true), + Arguments.of(ByteOrder.BIG_ENDIAN, false), Arguments.of(ByteOrder.BIG_ENDIAN, true)); + } + + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void testGpsVersionIdThroughPublicMetadataApi(final boolean littleEndian) throws IOException { + final ByteOrder order = littleEndian ? ByteOrder.LITTLE_ENDIAN : ByteOrder.BIG_ENDIAN; + final TiffImageMetadata metadata = (TiffImageMetadata) Imaging.getMetadata(createGpsMetadata(order)); + final TiffField version = metadata.findField(GpsTagConstants.GPS_TAG_GPS_VERSION_ID, true); + Assertions.assertNotNull(version, "GPSVersionID must be present in the GPS directory"); + Assertions.assertArrayEquals(new byte[] { 2, 3, 0, 0 }, version.getByteArrayValue()); + Assertions.assertEquals("N", metadata.findField(GpsTagConstants.GPS_TAG_GPS_LATITUDE_REF, true).getStringValue()); + } + + @ParameterizedTest + @MethodSource("readerOptions") + void testGpsVersionIdThroughReader(final ByteOrder order, final boolean strict) throws IOException { + final TiffContents contents = new TiffReader(strict).readDirectories(ByteSource.array(createGpsMetadata(order)), false, + FormatCompliance.getDefault()); + Assertions.assertEquals(2, contents.directories.size()); + final TiffDirectory gps = contents.directories.get(1); + Assertions.assertEquals(TiffDirectoryConstants.DIRECTORY_TYPE_GPS, gps.type); + Assertions.assertEquals("N", gps.findField(GpsTagConstants.GPS_TAG_GPS_LATITUDE_REF).getStringValue()); + Assertions.assertNotNull(gps.findField(GpsTagConstants.GPS_TAG_GPS_VERSION_ID), "GPSVersionID must not be skipped"); + Assertions.assertArrayEquals(new byte[] { 2, 3, 0, 0 }, gps.getFieldValue(GpsTagConstants.GPS_TAG_GPS_VERSION_ID, true)); + } + + @ParameterizedTest + @MethodSource("readerOptions") + void testNonGpsTagZeroRemainsSkipped(final ByteOrder order, final boolean strict) throws IOException { + final ByteBuffer buffer = ByteBuffer.allocate(38).order(order); + final byte marker = (byte) (order == ByteOrder.LITTLE_ENDIAN ? 'I' : 'M'); + buffer.put(marker).put(marker).putShort((short) 42).putInt(8); + buffer.putShort((short) 2); + // A malformed root entry must be skipped before interpreting its enormous count. + buffer.putShort((short) 0).putShort((short) 1).putInt(-1).putInt(0); + buffer.putShort((short) 0x010e).putShort((short) 2).putInt(2).put(new byte[] { 'X', 0, 0, 0 }); + buffer.putInt(0); + final TiffContents contents = new TiffReader(strict).readDirectories(ByteSource.array(buffer.array()), false, + FormatCompliance.getDefault()); + Assertions.assertEquals(1, contents.directories.size()); + final TiffDirectory root = contents.directories.get(0); + Assertions.assertNull(root.findField(GpsTagConstants.GPS_TAG_GPS_VERSION_ID)); + Assertions.assertEquals("X", root.findField(TiffTagConstants.TIFF_TAG_IMAGE_DESCRIPTION).getStringValue()); + } +} From 2a10b26744633c4d9b6d94b50b4243defa643e57 Mon Sep 17 00:00:00 2001 From: Qingyuan Zhu Date: Sat, 31 Oct 2026 04:02:18 +1100 Subject: [PATCH 2/2] IMAGING-371 Add GPS tag-zero boundary regression tests --- .../formats/tiff/TiffGpsVersionIdTest.java | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java b/src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java index 1c9feba1a..62f3a2c08 100644 --- a/src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java +++ b/src/test/java/org/apache/commons/imaging/formats/tiff/TiffGpsVersionIdTest.java @@ -100,4 +100,76 @@ void testNonGpsTagZeroRemainsSkipped(final ByteOrder order, final boolean strict Assertions.assertNull(root.findField(GpsTagConstants.GPS_TAG_GPS_VERSION_ID)); Assertions.assertEquals("X", root.findField(TiffTagConstants.TIFF_TAG_IMAGE_DESCRIPTION).getStringValue()); } + + /** + * Builds a root IFD pointing at a GPS IFD whose first entry is tag zero with an out-of-range offset/count, the + * exact shape the original blanket "tag == 0" skip was guarding against (IMAGING-371 comment: "can have invalid + * value lengths, which can cause OOM problems"). Since the fix now lets GPS tag zero through, the reader's + * existing offset/length bounds check (TiffReader#readDirectory, the {@code valueLength > entryMaxValueLength} + * branch) must still reject it instead of attempting an out-of-bounds read. + */ + private static byte[] createGpsDirectoryWithMalformedTagZero(final ByteOrder order) { + final ByteBuffer buffer = ByteBuffer.allocate(56).order(order); + final byte marker = (byte) (order == ByteOrder.LITTLE_ENDIAN ? 'I' : 'M'); + buffer.put(marker).put(marker).putShort((short) 42).putInt(8); + buffer.putShort((short) 1); + // Root GPSInfo pointer: LONG[1], pointing to the GPS IFD at offset 26. + buffer.putShort((short) 0x8825).putShort((short) 4).putInt(1).putInt(26); + buffer.putInt(0); + buffer.putShort((short) 2); + // GPSVersionID: BYTE, with a count so large the computed value length overflows the file size. + buffer.putShort((short) 0).putShort((short) 1).putInt(Integer.MAX_VALUE).putInt(0x7FFFFFFF); + // GPSLatitudeRef follows so traversal can continue past the malformed entry. + buffer.putShort((short) 1).putShort((short) 2).putInt(2).put(new byte[] { 'N', 0, 0, 0 }); + buffer.putInt(0); + return buffer.array(); + } + + @ParameterizedTest + @MethodSource("readerOptions") + void testMalformedGpsTagZeroOffsetIsRejectedNotSkipped(final ByteOrder order, final boolean strict) throws IOException { + final byte[] data = createGpsDirectoryWithMalformedTagZero(order); + if (strict) { + Assertions.assertThrows(IOException.class, + () -> new TiffReader(true).readDirectories(ByteSource.array(data), false, FormatCompliance.getDefault()), + "A GPSVersionID entry with an out-of-range offset/count must fail loudly in strict mode"); + return; + } + final TiffContents contents = Assertions.assertDoesNotThrow( + () -> new TiffReader(false).readDirectories(ByteSource.array(data), false, FormatCompliance.getDefault())); + final TiffDirectory gps = contents.directories.get(1); + Assertions.assertEquals(TiffDirectoryConstants.DIRECTORY_TYPE_GPS, gps.type); + Assertions.assertNull(gps.findField(GpsTagConstants.GPS_TAG_GPS_VERSION_ID), + "The corrupt GPSVersionID entry must be discarded, not read out-of-bounds"); + Assertions.assertEquals("N", gps.findField(GpsTagConstants.GPS_TAG_GPS_LATITUDE_REF).getStringValue(), + "The following entry must still be read normally"); + } + + /** + * GPS tag zero with an unrecognized field type code. {@link org.apache.commons.imaging.formats.tiff.fieldtypes.AbstractFieldType#getFieldType(int)} + * throws for unknown types, and TiffReader's existing catch-and-continue for that case must still apply to the + * GPS directory now that tag zero is no longer unconditionally skipped there. + */ + @ParameterizedTest + @MethodSource("readerOptions") + void testGpsTagZeroWithUnknownFieldTypeIsSkipped(final ByteOrder order, final boolean strict) throws IOException { + final ByteBuffer buffer = ByteBuffer.allocate(56).order(order); + final byte marker = (byte) (order == ByteOrder.LITTLE_ENDIAN ? 'I' : 'M'); + buffer.put(marker).put(marker).putShort((short) 42).putInt(8); + buffer.putShort((short) 1); + buffer.putShort((short) 0x8825).putShort((short) 4).putInt(1).putInt(26); + buffer.putInt(0); + buffer.putShort((short) 2); + // GPSVersionID with type code 0, which is not a recognized TIFF field type. + buffer.putShort((short) 0).putShort((short) 0).putInt(4).put(new byte[] { 2, 3, 0, 0 }); + buffer.putShort((short) 1).putShort((short) 2).putInt(2).put(new byte[] { 'N', 0, 0, 0 }); + buffer.putInt(0); + final TiffContents contents = new TiffReader(strict).readDirectories(ByteSource.array(buffer.array()), false, + FormatCompliance.getDefault()); + final TiffDirectory gps = contents.directories.get(1); + Assertions.assertEquals(TiffDirectoryConstants.DIRECTORY_TYPE_GPS, gps.type); + Assertions.assertNull(gps.findField(GpsTagConstants.GPS_TAG_GPS_VERSION_ID), + "An unrecognized field type must still be skipped regardless of directory type"); + Assertions.assertEquals("N", gps.findField(GpsTagConstants.GPS_TAG_GPS_LATITUDE_REF).getStringValue()); + } }