This is an automated email from the ASF dual-hosted git repository.
tballison pushed a commit to branch branch_3x
in repository https://gitbox.apache.org/repos/asf/tika.git
The following commit(s) were added to refs/heads/branch_3x by this push:
new 4af1cb0e31 TIKA-4812: MP4Parser skips oversized boxes instead of
allocating them (backport) (#3501)
4af1cb0e31 is described below
commit 4af1cb0e31484519be8c2434f29a034a654bf889
Author: Tim Allison <[email protected]>
AuthorDate: Mon Nov 2 12:24:12 2026 -0500
TIKA-4812: MP4Parser skips oversized boxes instead of allocating them
(backport) (#3501)
---
CHANGES.txt | 5 +
.../java/org/apache/tika/parser/mp4/MP4Parser.java | 23 +++-
.../org/apache/tika/parser/mp4/TikaMp4Reader.java | 136 +++++++++++++++++++++
.../tika/parser/mp4/boxes/TikaUserDataBox.java | 8 ++
.../org/apache/tika/parser/mp4/MP4ParserTest.java | 74 +++++++++++
.../tika/parser/mp4/boxes/TikaUserDataBoxTest.java | 52 ++++++++
6 files changed, 296 insertions(+), 2 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index a1360a1b57..27a0e3eb73 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,10 @@
Release 3.4.0 - ???
+ * MP4Parser no longer allocates a box's declared size up front: a box larger
than
+ maxBoxSize (default 100 MB, configurable) or than the file is skipped and
recorded as
+ a warning, so a crafted box header cannot force a multi-GB allocation. A
malformed
+ udta sub-box length no longer throws IllegalArgumentException (TIKA-4812).
+
* The tika-app GUI's Formatted Text view no longer lets a document make Swing
construct classes or fetch URLs: only plain elements and attributes reach
it, and
remote images are not loaded (TIKA-4992).
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/MP4Parser.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/MP4Parser.java
index 0b7e30b2aa..f165e206b2 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/MP4Parser.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/MP4Parser.java
@@ -33,7 +33,6 @@ import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import com.drew.imaging.mp4.Mp4Reader;
import com.drew.metadata.Directory;
import com.drew.metadata.MetadataException;
import com.drew.metadata.mp4.Mp4BoxHandler;
@@ -43,6 +42,7 @@ import com.drew.metadata.mp4.media.Mp4VideoDirectory;
import org.xml.sax.ContentHandler;
import org.xml.sax.SAXException;
+import org.apache.tika.config.Field;
import org.apache.tika.exception.RuntimeSAXException;
import org.apache.tika.exception.TikaException;
import org.apache.tika.io.TemporaryResources;
@@ -76,6 +76,11 @@ public class MP4Parser implements Parser {
private static final MediaType AUDIO_MP4 = MediaType.audio("mp4");
private static final int MAX_ERROR_MESSAGES = 100;
+
+ //an accepted box with a larger payload is skipped, not loaded (TIKA-4812)
+ private static final long DEFAULT_MAX_BOX_SIZE = 100L * 1024L * 1024L;
+
+ private long maxBoxSize = DEFAULT_MAX_BOX_SIZE;
static {
// All types should be 4 bytes long, space padded as needed
typesMap.put(MediaType.audio("mp4"), Arrays.asList("M4A ", "M4B ",
"F4A ", "F4B "));
@@ -94,6 +99,19 @@ public class MP4Parser implements Parser {
return SUPPORTED_TYPES;
}
+ /**
+ * The maximum payload, in bytes, of an MP4 box that will be read into
memory;
+ * larger boxes are skipped.
+ */
+ public long getMaxBoxSize() {
+ return maxBoxSize;
+ }
+
+ @Field
+ public void setMaxBoxSize(long maxBoxSize) {
+ this.maxBoxSize = maxBoxSize;
+ }
+
public void parse(InputStream stream, ContentHandler handler, Metadata
metadata,
ParseContext context) throws IOException, SAXException,
TikaException {
@@ -107,7 +125,8 @@ public class MP4Parser implements Parser {
com.drew.metadata.Metadata mp4Metadata = new
com.drew.metadata.Metadata();
Mp4BoxHandler boxHandler = new TikaMp4BoxHandler(mp4Metadata,
metadata, xhtml);
try {
- Mp4Reader.extract(is, boxHandler);
+ TikaMp4Reader.extract(is, boxHandler, maxBoxSize,
+ Files.size(tstream.getPath()));
} catch (RuntimeSAXException e) {
throw (SAXException) e.getCause();
}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4Reader.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4Reader.java
new file mode 100644
index 0000000000..3f45d941bb
--- /dev/null
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4Reader.java
@@ -0,0 +1,136 @@
+/*
+ * 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
+ *
+ * http://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.tika.parser.mp4;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import com.drew.imaging.mp4.Mp4Handler;
+import com.drew.lang.StreamReader;
+import com.drew.metadata.mp4.Mp4BoxHandler;
+import com.drew.metadata.mp4.Mp4Context;
+import com.drew.metadata.mp4.Mp4MediaHandler;
+
+/**
+ * A size-bounded copy of metadata-extractor 2.22.0's
com.drew.imaging.mp4.Mp4Reader.
+ * <p>
+ * The library reader does {@code new byte[(int) boxSize - 8]} for every box a
handler
+ * accepts, with {@code boxSize} taken from the file and capped only at
+ * {@code Integer.MAX_VALUE}, and {@code StreamReader.getBytes} allocates
before checking
+ * how much data is present, so one crafted box header forces a multi-GB
allocation.
+ * This walk is the library's except that an accepted box whose payload exceeds
+ * {@code maxBoxSize}, or the input, is skipped instead of read. See TIKA-4812.
+ */
+final class TikaMp4Reader {
+
+ //same limit as the library's walk
+ private static final int MAX_BOX_DEPTH = 100;
+
+ private TikaMp4Reader() {
+ }
+
+ /**
+ * @param inputLength total input length in bytes, or -1 if unknown
+ */
+ static void extract(InputStream inputStream, Mp4BoxHandler handler, long
maxBoxSize,
+ long inputLength) {
+ StreamReader reader = new StreamReader(inputStream);
+ reader.setMotorolaByteOrder(true);
+ processBoxes(reader, -1, 0, handler, new Mp4Context(), maxBoxSize,
inputLength);
+ }
+
+ private static void processBoxes(StreamReader reader, long atomEnd, int
depth,
+ Mp4Handler<?> handler, Mp4Context context,
+ long maxBoxSize, long inputLength) {
+ try {
+ if (depth > MAX_BOX_DEPTH) {
+ handler.addError("Box nesting too deep.");
+ reader.skip(atomEnd - reader.getPosition());
+ return;
+ }
+ while (atomEnd == -1 || reader.getPosition() < atomEnd) {
+ long boxSize = reader.getUInt32();
+ String boxType = reader.getString(4);
+ //4 bytes size + 4 bytes type, plus 8 more when a 64-bit
largesize follows
+ int headerSize = boxSize == 1 ? 16 : 8;
+ if (headerSize == 16) {
+ boxSize = reader.getInt64();
+ }
+ if (boxSize > Integer.MAX_VALUE) {
+ handler.addError("Box size too large.");
+ break;
+ }
+ if (boxSize < headerSize) {
+ handler.addError("Box size too small.");
+ break;
+ }
+
+ long payloadLength = boxSize - headerSize;
+ if (acceptContainer(handler, boxType)) {
+ processBoxes(reader, reader.getPosition() + payloadLength,
depth + 1,
+ processBox(handler, boxType, null, boxSize,
context), context,
+ maxBoxSize, inputLength);
+ } else if (acceptBox(handler, boxType)) {
+ boolean tooLarge = payloadLength > maxBoxSize;
+ boolean beyondInput = inputLength >= 0
+ && reader.getPosition() + payloadLength >
inputLength;
+ if (tooLarge || beyondInput) {
+ handler.addError("MP4 box '" + boxType + "' payload ("
+ payloadLength
+ + " bytes) exceeds the "
+ + (tooLarge ? "maximum of " + maxBoxSize + "
bytes" : "input size")
+ + "; skipping.");
+ reader.skip(payloadLength);
+ } else {
+ handler = processBox(handler, boxType,
+ reader.getBytes((int) payloadLength), boxSize,
context);
+ }
+ } else {
+ reader.skip(payloadLength);
+ }
+ }
+ } catch (IOException e) {
+ handler.addError(e.getMessage());
+ }
+ }
+
+ //Mp4Handler's accept/process methods are protected; Mp4BoxHandler and the
Mp4MediaHandler
+ //track handlers swapped in on 'hdlr' widen them to public. Any other
handler type accepts
+ //nothing. processBox(type, null, ...) is what the library's
processContainer does.
+
+ private static boolean acceptContainer(Mp4Handler<?> handler, String type)
{
+ if (handler instanceof Mp4BoxHandler) {
+ return ((Mp4BoxHandler) handler).shouldAcceptContainer(type);
+ }
+ return handler instanceof Mp4MediaHandler
+ && ((Mp4MediaHandler<?>) handler).shouldAcceptContainer(type);
+ }
+
+ private static boolean acceptBox(Mp4Handler<?> handler, String type) {
+ if (handler instanceof Mp4BoxHandler) {
+ return ((Mp4BoxHandler) handler).shouldAcceptBox(type);
+ }
+ return handler instanceof Mp4MediaHandler
+ && ((Mp4MediaHandler<?>) handler).shouldAcceptBox(type);
+ }
+
+ private static Mp4Handler<?> processBox(Mp4Handler<?> handler, String
type, byte[] payload,
+ long boxSize, Mp4Context context)
throws IOException {
+ return handler instanceof Mp4BoxHandler
+ ? ((Mp4BoxHandler) handler).processBox(type, payload, boxSize,
context)
+ : ((Mp4MediaHandler<?>) handler).processBox(type, payload,
boxSize, context);
+ }
+}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/boxes/TikaUserDataBox.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/boxes/TikaUserDataBox.java
index 4fd264aed3..b8d3b72961 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/boxes/TikaUserDataBox.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/boxes/TikaUserDataBox.java
@@ -119,6 +119,10 @@ public class TikaUserDataBox {
//this handles "free" types...not sure if there are others?
//will throw IOException if no ilist is found
while (! subType.equals(ILST)) {
+ //len < 8 is a negative skip: IllegalArgumentException, which
escapes the reader (TIKA-4812)
+ if (len < 8L || len >= Integer.MAX_VALUE) {
+ return;
+ }
reader.skip(len - 8);
len = reader.getUInt32();
subType = reader.getString(4, StandardCharsets.ISO_8859_1);
@@ -156,6 +160,10 @@ public class TikaUserDataBox {
} else if ("cpil".equals(fieldName)) {
int compilationId = (int)reader.getByte();
metadata.set(XMPDM.COMPILATION, compilationId);
+ //totalRead counts toRead; skip the rest to stay aligned
+ if (toRead > 1) {
+ reader.skip(toRead - 1L);
+ }
} else if ("trkn".equals(fieldName)) {
if (toRead == 8) {
long numA = reader.getUInt32();
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java
index f0e5b75300..e67d76c559 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java
@@ -17,9 +17,15 @@
package org.apache.tika.parser.mp4;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -34,6 +40,7 @@ import com.drew.metadata.mp4.media.Mp4VideoDirectory;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.xml.sax.ContentHandler;
+import org.xml.sax.helpers.DefaultHandler;
import org.apache.tika.TikaTest;
import org.apache.tika.io.TikaInputStream;
@@ -43,6 +50,7 @@ import org.apache.tika.metadata.XMP;
import org.apache.tika.metadata.XMPDM;
import org.apache.tika.parser.ParseContext;
import org.apache.tika.sax.BodyContentHandler;
+import org.apache.tika.sax.XHTMLContentHandler;
/**
@@ -319,4 +327,70 @@ public class MP4ParserTest extends TikaTest {
assertEquals(-98.7654,
Double.parseDouble(metadata.get(TikaCoreProperties.LONGITUDE)), 0.00001);
assertEquals(10.5,
Double.parseDouble(metadata.get(TikaCoreProperties.ALTITUDE)), 0.00001);
}
+
+ @Test
+ public void testOversizedBoxIsSkippedNotAllocated() throws Exception {
+ //the library reader would do new byte[payload] for an accepted box.
TIKA-4812
+ byte[] ftyp = ftypBox(24);
+ assertNull(majorBrand(ftyp, 8L)); //cap below the 16-byte
payload: skipped
+ assertEquals("isom", majorBrand(ftyp, 1000L)); //above it: read
+ }
+
+ @Test
+ public void testBoxClaimingMoreThanTheInputIsSkipped() throws Exception {
+ //a 24-byte file whose ftyp claims ~1.5 GB, under the cap only because
the cap is raised
+ byte[] ftyp = ftypBox(1_500_000_000);
+ Mp4Directory dir = walk(ftyp, Long.MAX_VALUE);
+ assertTrue(dir.getErrors().toString().contains("exceeds the input
size"),
+ dir.getErrors().toString());
+ }
+
+ @Test
+ public void testNestedContainerRecursionIsBounded() throws Exception {
+ int depth = 100_000;
+ ByteBuffer buf = ByteBuffer.allocate(depth * 8);
+ for (int k = 0; k < depth; k++) {
+ buf.putInt(8 * (depth - k));
+ buf.put("moov".getBytes(StandardCharsets.ISO_8859_1));
+ }
+ assertDoesNotThrow(() -> walk(buf.array(), 1000L));
+ }
+
+ @Test
+ public void testLargeSizeBoxHeaderAccounting() throws Exception {
+ //a 64-bit largesize box has a 16-byte header; the ftyp after it must
still parse
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ bos.write(new byte[]{0, 0, 0, 1});
+ bos.write("udta".getBytes(StandardCharsets.ISO_8859_1));
+ bos.write(new byte[]{0, 0, 0, 0, 0, 0, 0, 24});
+ bos.write(new byte[]{0, 0, 0, 8});
+ bos.write("free".getBytes(StandardCharsets.ISO_8859_1));
+ bos.write(ftypBox(24));
+ assertEquals("isom", majorBrand(bos.toByteArray(), 1000L));
+ }
+
+ private static String majorBrand(byte[] boxes, long maxBoxSize) throws
Exception {
+ Mp4Directory dir = walk(boxes, maxBoxSize);
+ return dir == null ? null :
dir.getString(Mp4Directory.TAG_MAJOR_BRAND);
+ }
+
+ private static Mp4Directory walk(byte[] boxes, long maxBoxSize) {
+ com.drew.metadata.Metadata mp4Metadata = new
com.drew.metadata.Metadata();
+ Metadata tikaMetadata = new Metadata();
+ XHTMLContentHandler xhtml = new XHTMLContentHandler(new
DefaultHandler(), tikaMetadata);
+ TikaMp4BoxHandler handler = new TikaMp4BoxHandler(mp4Metadata,
tikaMetadata, xhtml);
+ TikaMp4Reader.extract(new ByteArrayInputStream(boxes), handler,
maxBoxSize, boxes.length);
+ return mp4Metadata.getFirstDirectoryOfType(Mp4Directory.class);
+ }
+
+ private static byte[] ftypBox(int declaredSize) {
+ ByteBuffer buf = ByteBuffer.allocate(24);
+ buf.putInt(declaredSize);
+ buf.put("ftyp".getBytes(StandardCharsets.ISO_8859_1));
+ buf.put("isom".getBytes(StandardCharsets.ISO_8859_1)); //major brand
+ buf.putInt(0);
+ buf.put("mp41".getBytes(StandardCharsets.ISO_8859_1));
+ buf.put("mp42".getBytes(StandardCharsets.ISO_8859_1));
+ return buf.array();
+ }
}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/boxes/TikaUserDataBoxTest.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/boxes/TikaUserDataBoxTest.java
new file mode 100644
index 0000000000..1980d04fca
--- /dev/null
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/boxes/TikaUserDataBoxTest.java
@@ -0,0 +1,52 @@
+/*
+ * 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
+ *
+ * http://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.tika.parser.mp4.boxes;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+
+import org.junit.jupiter.api.Test;
+import org.xml.sax.helpers.DefaultHandler;
+
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.sax.XHTMLContentHandler;
+
+public class TikaUserDataBoxTest {
+
+ @Test
+ public void testIlstSearchMalformedSubBoxLength() {
+ //a sub-box length below 8 made skip(len - 8) negative: an
IllegalArgumentException
+ //that escaped the MP4 reader. TIKA-4812
+ ByteBuffer buf = ByteBuffer.allocate(40);
+ buf.putInt(40);
+ buf.put("meta".getBytes(StandardCharsets.ISO_8859_1));
+ buf.putInt(0);
+ buf.putInt(20); //lengthToStartOfList
= 16: no skip
+ buf.put("hdlr".getBytes(StandardCharsets.ISO_8859_1));
+ buf.putInt(0);
+ buf.putInt(0);
+ buf.put("mdir".getBytes(StandardCharsets.ISO_8859_1));
+ buf.putInt(4); //malformed sub-box
length
+ buf.put("free".getBytes(StandardCharsets.ISO_8859_1)); //not ilst:
enters the search
+
+ Metadata metadata = new Metadata();
+ XHTMLContentHandler xhtml = new XHTMLContentHandler(new
DefaultHandler(), metadata);
+ assertDoesNotThrow(() -> new TikaUserDataBox("udta", buf.array(),
metadata, xhtml));
+ }
+}
