This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch TIKA-4812-improve-media-file-robustness in repository https://gitbox.apache.org/repos/asf/tika.git
commit 327e2a02da961bef2a6a35e62a8ceb5e447e086a Author: tallison <[email protected]> AuthorDate: Mon Aug 10 10:57:11 2026 -0400 TIKA-4812: harden MP4 parsing against crafted box structures --- .../java/org/apache/tika/parser/mp4/MP4Parser.java | 24 +++- .../org/apache/tika/parser/mp4/TikaMp4Reader.java | 126 +++++++++++++++++++++ .../tika/parser/mp4/TikaMp4SoundHandler.java | 15 ++- .../tika/parser/mp4/boxes/TikaUserDataBox.java | 6 + .../org/apache/tika/parser/mp4/MP4ParserTest.java | 65 +++++++++++ .../tika/parser/mp4/boxes/TikaUserDataBoxTest.java | 59 ++++++++++ 6 files changed, 292 insertions(+), 3 deletions(-) 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 b146a8bab4..e0e7b6cf83 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 @@ -31,7 +31,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; @@ -77,6 +76,14 @@ public class MP4Parser implements Parser { private static final MediaType AUDIO_MP4 = MediaType.audio("mp4"); private static final int MAX_ERROR_MESSAGES = 100; + + //an accepted MP4 box whose declared payload exceeds this is skipped rather than + //loaded, so a crafted box size cannot force a multi-GB allocation. Cover art and + //other legitimate metadata boxes are well under this; configurable if a real file + //needs more. See TikaMp4Reader and 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 ")); @@ -95,6 +102,19 @@ public class MP4Parser implements Parser { return SUPPORTED_TYPES; } + /** + * The maximum declared payload, in bytes, of an accepted MP4 box that will be + * read into memory; larger boxes are skipped. Guards against a crafted box size + * forcing a multi-GB allocation. + */ + public long getMaxBoxSize() { + return maxBoxSize; + } + + public void setMaxBoxSize(long maxBoxSize) { + this.maxBoxSize = maxBoxSize; + } + public void parse(TikaInputStream tis, ContentHandler handler, Metadata metadata, ParseContext context) throws IOException, SAXException, TikaException { @@ -106,7 +126,7 @@ public class MP4Parser implements Parser { //we used to spool to disk and then read from that with sannies parser. //we think that drewnoakes' parser streams the data so we don't need to spool try { - Mp4Reader.extract(tis, boxHandler); + TikaMp4Reader.extract(tis, boxHandler, maxBoxSize); } 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..4ea01bed47 --- /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,126 @@ +/* + * 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 reimplementation of com.drew.imaging.mp4.Mp4Reader. + * <p> + * The metadata-extractor reader eagerly does {@code new byte[(int) boxSize - 8]} + * for every box a handler accepts, with {@code boxSize} attacker-controlled and + * capped only at {@code Integer.MAX_VALUE} (~2GB), and {@code StreamReader.getBytes} + * allocates before checking how much data is actually present. A single crafted + * box header therefore forces a multi-GB allocation. This reader is identical to + * the library's box walk except that an accepted box whose payload exceeds + * {@code maxBoxSize} is skipped (a lazy stream advance, no allocation) instead of + * being read. Boxes the handler does not accept were already skipped by the + * library, so this only bounds the boxes we opt into. See TIKA-4812. + */ +final class TikaMp4Reader { + + private TikaMp4Reader() { + } + + static void extract(InputStream inputStream, Mp4BoxHandler handler, long maxBoxSize) { + StreamReader reader = new StreamReader(inputStream); + reader.setMotorolaByteOrder(true); + processBoxes(reader, -1, handler, new Mp4Context(), maxBoxSize); + } + + private static void processBoxes(StreamReader reader, long atomEnd, Mp4Handler<?> handler, + Mp4Context context, long maxBoxSize) { + try { + while (atomEnd == -1 || reader.getPosition() < atomEnd) { + long boxSize = reader.getUInt32(); + String boxType = reader.getString(4); + boolean isLargeSize = boxSize == 1; + if (isLargeSize) { + boxSize = reader.getInt64(); + } + if (boxSize > Integer.MAX_VALUE) { + handler.addError("Box size too large."); + break; + } + if (boxSize < 8) { + handler.addError("Box size too small."); + break; + } + + if (acceptContainer(handler, boxType)) { + processBoxes(reader, boxSize + reader.getPosition() - 8, + processBox(handler, boxType, null, boxSize, context), context, + maxBoxSize); + } else if (acceptBox(handler, boxType)) { + long payloadLength = boxSize - 8; + if (payloadLength > maxBoxSize) { + handler.addError("MP4 box '" + boxType + "' payload (" + payloadLength + + " bytes) exceeds the maximum of " + maxBoxSize + + " bytes; skipping."); + reader.skip(payloadLength); + } else { + handler = processBox(handler, boxType, + reader.getBytes((int) payloadLength), boxSize, context); + } + } else if (isLargeSize) { + if (boxSize < 16) { + break; + } + reader.skip(boxSize - 16); + } else { + reader.skip(boxSize - 8); + } + } + } catch (IOException e) { + handler.addError(e.getMessage() == null ? "IOException reading MP4 boxes" + : e.getMessage()); + } + } + + //the box walk holds handlers as Mp4Handler, whose accept/process methods are + //protected; every concrete handler in play (Mp4BoxHandler-rooted, or an + //Mp4MediaHandler track handler swapped in on 'hdlr') widens them to public, + //so dispatch through whichever of the two families the instance belongs to. + //A container's handler is obtained with processBox(type, null, ...), which is + //exactly what the library's protected processContainer does. + + private static boolean acceptContainer(Mp4Handler<?> handler, String type) { + return handler instanceof Mp4BoxHandler + ? ((Mp4BoxHandler) handler).shouldAcceptContainer(type) + : ((Mp4MediaHandler<?>) handler).shouldAcceptContainer(type); + } + + private static boolean acceptBox(Mp4Handler<?> handler, String type) { + return handler instanceof Mp4BoxHandler + ? ((Mp4BoxHandler) handler).shouldAcceptBox(type) + : ((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/TikaMp4SoundHandler.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4SoundHandler.java index 770c4cd722..624c3aa56b 100644 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4SoundHandler.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/mp4/TikaMp4SoundHandler.java @@ -103,12 +103,25 @@ class TikaMp4SoundHandler extends Mp4SoundHandler { return 36; } + //real files nest 'wave' at most one level; this only bounds crafted input, + //where a deep chain of nested 'wave' boxes would otherwise recurse until the + //stack overflows (an uncaught Error, not caught by Mp4Reader or CompositeParser). + //See TIKA-4812. + private static final int MAX_BOX_DEPTH = 10; + /** * Scans the child boxes of a sample entry for an 'esds' box and returns * its average bitrate, or 0 if there is none. QuickTime version 1/2 * entries may nest the 'esds' inside a 'wave' extension box. */ private static int findEsdsAverageBitRate(byte[] b, int pos, int end) { + return findEsdsAverageBitRate(b, pos, end, 0); + } + + private static int findEsdsAverageBitRate(byte[] b, int pos, int end, int depth) { + if (depth > MAX_BOX_DEPTH) { + return 0; + } while (pos >= 0 && pos + 8 <= end) { long size = EndianUtils.getUIntBE(b, pos); if (size < 8 || size > end - pos) { @@ -119,7 +132,7 @@ class TikaMp4SoundHandler extends Mp4SoundHandler { return readEsdsAverageBitRate(b, pos + 8, pos + (int) size); } if ("wave".equals(type)) { - int nested = findEsdsAverageBitRate(b, pos + 8, pos + (int) size); + int nested = findEsdsAverageBitRate(b, pos + 8, pos + (int) size, depth + 1); if (nested > 0) { return nested; } 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 72fd54ca4b..1bf375c6de 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 @@ -123,6 +123,12 @@ 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)) { + //re-validate each re-read length: len < 8 makes skip(len - 8) negative, + //which throws IllegalArgumentException (not IOException, so it escapes + //MP4Reader). See 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); 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 97197bcd4e..ad627a1c43 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 @@ -21,7 +21,9 @@ 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; @@ -39,6 +41,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; @@ -51,6 +54,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; /** @@ -432,6 +436,67 @@ public class MP4ParserTest extends TikaTest { assertNull(tikaMetadata.get(QuickTime.STILL_IMAGE_TIME)); } + @Test + public void testStsdNestedWaveRecursion() throws Exception { + //a crafted sound sample description whose child boxes are a deep chain of + //nested 'wave' boxes used to recurse in findEsdsAverageBitRate until the + //stack overflowed (an uncaught Error, not caught by Mp4Reader or + //CompositeParser); the handler must bound the box nesting depth. TIKA-4812 + int depth = 100_000; + int childLen = depth * 8; + ByteBuffer buf = ByteBuffer.allocate(44 + childLen); //big-endian by default + buf.putInt(0); //version and flags + buf.putInt(1); //entry count + buf.putInt(36 + childLen); //sample entry size + buf.put("mp4a".getBytes(StandardCharsets.ISO_8859_1)); + buf.position(44); //leave the 28 fixed sound fields zero (version 0 -> 36 byte entry) + for (int k = 0; k < depth; k++) { + buf.putInt(8 * (depth - k)); //'wave' box size, shrinking to the chain end + buf.put("wave".getBytes(StandardCharsets.ISO_8859_1)); + } + + Metadata tikaMetadata = new Metadata(); + TikaMp4SoundHandler handler = new TikaMp4SoundHandler(new com.drew.metadata.Metadata(), + new Mp4Context(), tikaMetadata); + //must return without a StackOverflowError, and find no bitrate + handler.processBox("stsd", buf.array(), buf.array().length, new Mp4Context()); + assertNull(tikaMetadata.get(Audio.BITRATE)); + } + + @Test + public void testOversizedBoxIsSkippedNotAllocated() throws Exception { + //an accepted box whose declared payload exceeds the cap must be skipped (a + //lazy stream advance, no allocation) rather than read into a byte[]; the + //metadata-extractor reader would instead do new byte[(int) boxSize - 8]. + //Use ftyp, an accepted top-level box with an observable side effect (the + //major brand). Payload is 16 bytes. TIKA-4812 + byte[] ftyp = ftypBox(); + assertNull(majorBrand(ftyp, 8L)); //cap below the payload -> skipped + assertEquals("isom", majorBrand(ftyp, 1000L)); //cap above it -> read and processed + } + + private static String majorBrand(byte[] boxes, long maxBoxSize) throws Exception { + 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, new ParseContext()); + TikaMp4Reader.extract(new ByteArrayInputStream(boxes), handler, maxBoxSize); + Mp4Directory dir = mp4Metadata.getFirstDirectoryOfType(Mp4Directory.class); + return dir == null ? null : dir.getString(Mp4Directory.TAG_MAJOR_BRAND); + } + + private static byte[] ftypBox() { + ByteBuffer buf = ByteBuffer.allocate(24); //big-endian by default + buf.putInt(24); //box size + buf.put("ftyp".getBytes(StandardCharsets.ISO_8859_1)); + buf.put("isom".getBytes(StandardCharsets.ISO_8859_1)); //major brand + buf.putInt(0); //minor version + buf.put("mp41".getBytes(StandardCharsets.ISO_8859_1)); //compatible brand + buf.put("mp42".getBytes(StandardCharsets.ISO_8859_1)); //compatible brand + return buf.array(); + } + @Test public void testUdtaLocation() throws Exception { //the udta "(c)xyz" ISO 6709 location is mapped to geo:lat/geo:long, and its 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..eb3c4898ae --- /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,59 @@ +/* + * 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.parser.ParseContext; +import org.apache.tika.sax.XHTMLContentHandler; + +public class TikaUserDataBoxTest { + + /** + * A meta/mdir udta whose ilst-search hits a sub-box declaring a length below + * the 8-byte header used to reach the next box: {@code reader.skip(len - 8)} + * would be a negative skip, which throws {@link IllegalArgumentException} (not + * IOException, so it escapes MP4Reader's catch). The box must consume it + * silently instead. See TIKA-4812. + */ + @Test + public void testIlstSearchMalformedSubBoxLength() { + ByteBuffer buf = ByteBuffer.allocate(40); //big-endian by default + buf.putInt(40); //meta box size (>4) + buf.put("meta".getBytes(StandardCharsets.ISO_8859_1)); + buf.putInt(0); //version and flags + buf.putInt(20); //-> lengthToStartOfList = 16, so no skip + buf.put("hdlr".getBytes(StandardCharsets.ISO_8859_1)); + buf.putInt(0); + buf.putInt(0); + buf.put("mdir".getBytes(StandardCharsets.ISO_8859_1)); //handler subtype -> MDIR path + buf.putInt(4); //malformed sub-box length (< 8) + buf.put("free".getBytes(StandardCharsets.ISO_8859_1)); //!= ilst -> enters the search loop + + Metadata metadata = new Metadata(); + XHTMLContentHandler xhtml = new XHTMLContentHandler(new DefaultHandler(), metadata); + assertDoesNotThrow(() -> + new TikaUserDataBox("udta", buf.array(), metadata, xhtml, new ParseContext())); + } +}
