From 649607f1fc76dd90daad3a373f868efdfca554df Mon Sep 17 00:00:00 2001 From: Acts1631 <69813585+acts-1631@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:32:51 -0400 Subject: [PATCH] Limit Matroska EBML element allocations (#1456) MasterElement permits up to one million elements per read. Segment metadata sections each start a fresh counter, allowing small files with empty elements to exhaust memory during parsing. Share one count across segment sections and seek-head lookups. Bound the count by file size, with a minimum for small valid files. --- taglib/matroska/ebml/ebmlmasterelement.cpp | 18 +++++++++++- taglib/matroska/ebml/ebmlmasterelement.h | 1 + taglib/matroska/ebml/ebmlmksegment.cpp | 34 ++++++++++++---------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/taglib/matroska/ebml/ebmlmasterelement.cpp b/taglib/matroska/ebml/ebmlmasterelement.cpp index 836f9003..85b4d0c2 100644 --- a/taglib/matroska/ebml/ebmlmasterelement.cpp +++ b/taglib/matroska/ebml/ebmlmasterelement.cpp @@ -104,6 +104,11 @@ bool EBML::MasterElement::read(File &file, int depth) return read(file, depth, elementCount); } +bool EBML::MasterElement::read(File &file, unsigned int &elementCount) +{ + return read(file, 0, elementCount); +} + bool EBML::MasterElement::read(File &file, int depth, unsigned int &elementCount) { static constexpr int MAX_EBML_DEPTH = 64; @@ -112,6 +117,17 @@ bool EBML::MasterElement::read(File &file, int depth, unsigned int &elementCount // elements, so a 50000 whole-file budget rejects ordinary media outright. static constexpr int MAX_EBML_ELEMENT_COUNT = 1000000; static constexpr int MAX_EBML_ELEMENT_COUNT_PER_LEVEL = 50000; + static constexpr offset_t MIN_EBML_ELEMENT_COUNT = 1024; + static constexpr offset_t BYTES_PER_EBML_ELEMENT = 32; + // A zero-length element can occupy only two bytes on disk but takes a heap + // allocation and a list node in memory. Bound the element budget by the + // input length, with a minimum for small but valid media files. + const offset_t fileLength = file.length(); + if(fileLength < 0) + return false; + const auto maxElementCount = static_cast( + std::min(MAX_EBML_ELEMENT_COUNT, + std::max(MIN_EBML_ELEMENT_COUNT, fileLength / BYTES_PER_EBML_ELEMENT))); if(depth > MAX_EBML_DEPTH) { debug("EBML: Maximum nesting depth exceeded"); return false; @@ -119,7 +135,7 @@ bool EBML::MasterElement::read(File &file, int depth, unsigned int &elementCount const offset_t maxOffset = file.tell() + dataSize; std::unique_ptr element; while((element = findNextElement(file, maxOffset))) { - if(elementCount >= MAX_EBML_ELEMENT_COUNT || + if(elementCount >= maxElementCount || elements.size() >= MAX_EBML_ELEMENT_COUNT_PER_LEVEL) { debug("EBML: Maximum element count exceeded"); return false; diff --git a/taglib/matroska/ebml/ebmlmasterelement.h b/taglib/matroska/ebml/ebmlmasterelement.h index 175fc7b5..67a8688a 100644 --- a/taglib/matroska/ebml/ebmlmasterelement.h +++ b/taglib/matroska/ebml/ebmlmasterelement.h @@ -41,6 +41,7 @@ namespace TagLib offset_t getOffset() const; bool read(File &file) override; + bool read(File &file, unsigned int &elementCount); ByteVector render() override; void appendElement(std::unique_ptr &&element); std::list>::iterator begin(); diff --git a/taglib/matroska/ebml/ebmlmksegment.cpp b/taglib/matroska/ebml/ebmlmksegment.cpp index 453289fd..0331439f 100644 --- a/taglib/matroska/ebml/ebmlmksegment.cpp +++ b/taglib/matroska/ebml/ebmlmksegment.cpp @@ -38,7 +38,8 @@ namespace { template std::unique_ptr readElementAt(File &file, offset_t offset, - offset_t maxOffset) + offset_t maxOffset, + unsigned int &elementCount) { if(offset < 0 || offset >= maxOffset) { return nullptr; @@ -51,7 +52,7 @@ std::unique_ptr readElementAt(File &file, } auto typed = EBML::element_cast(std::move(element)); - if(!typed || !typed->read(file)) { + if(!typed || !typed->read(file, elementCount)) { return nullptr; } return typed; @@ -94,11 +95,12 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) const bool skipCues = file.readOnly() && scanLimit < dataSize; MasterElement *pendingPaddingTarget = nullptr; offset_t accumulatedPadding = 0; + unsigned int elementCount = 0; std::unique_ptr element; while((element = findNextElement(file, maxOffset, maxScanOffset))) { if(const Id id = element->getId(); id == Id::MkSeekHead) { seekHead = element_cast(std::move(element)); - if(!seekHead->read(file)) + if(!seekHead->read(file, elementCount)) return false; // We have a seek head, let's use it for faster access to the other elements if(const auto elementAfterSeekHead = findNextElement(file, maxOffset, maxScanOffset); @@ -138,7 +140,7 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) if(chainedSeekHeadsFollowed++ >= MAX_CHAINED_SEEKHEADS) break; auto chained = readElementAt( - file, absoluteOffset, maxOffset); + file, absoluteOffset, maxOffset, elementCount); if(!chained) break; if(const auto parsed = chained->parse(segDataOffset)) { @@ -150,35 +152,35 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) case Id::MkCues: if(!skipCues) { if(!((cues = readElementAt( - file, absoluteOffset, maxOffset)))) + file, absoluteOffset, maxOffset, elementCount)))) return false; } break; case Id::MkInfo: if(!((info = readElementAt( - file, absoluteOffset, maxOffset)))) + file, absoluteOffset, maxOffset, elementCount)))) return false; break; case Id::MkTracks: if(!((tracks = readElementAt( - file, absoluteOffset, maxOffset)))) + file, absoluteOffset, maxOffset, elementCount)))) return false; break; case Id::MkTags: if(!((tags = readElementAt( - file, absoluteOffset, maxOffset)))) + file, absoluteOffset, maxOffset, elementCount)))) return false; accumulateVoidPadding(tags.get()); break; case Id::MkAttachments: if(!((attachments = readElementAt( - file, absoluteOffset, maxOffset)))) + file, absoluteOffset, maxOffset, elementCount)))) return false; accumulateVoidPadding(attachments.get()); break; case Id::MkChapters: if(!((chapters = readElementAt( - file, absoluteOffset, maxOffset)))) + file, absoluteOffset, maxOffset, elementCount)))) return false; accumulateVoidPadding(chapters.get()); break; @@ -200,7 +202,7 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) accumulatedPadding = 0; if(!skipCues) { cues = element_cast(std::move(element)); - if(!cues->read(file)) + if(!cues->read(file, elementCount)) return false; } else { @@ -211,21 +213,21 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) pendingPaddingTarget = nullptr; accumulatedPadding = 0; info = element_cast(std::move(element)); - if(!info->read(file)) + if(!info->read(file, elementCount)) return false; } else if(id == Id::MkTracks) { pendingPaddingTarget = nullptr; accumulatedPadding = 0; tracks = element_cast(std::move(element)); - if(!tracks->read(file)) + if(!tracks->read(file, elementCount)) return false; } else if(id == Id::MkTags) { pendingPaddingTarget = nullptr; accumulatedPadding = 0; tags = element_cast(std::move(element)); - if(!tags->read(file)) + if(!tags->read(file, elementCount)) return false; pendingPaddingTarget = tags.get(); } @@ -233,7 +235,7 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) pendingPaddingTarget = nullptr; accumulatedPadding = 0; attachments = element_cast(std::move(element)); - if(!attachments->read(file)) + if(!attachments->read(file, elementCount)) return false; pendingPaddingTarget = attachments.get(); } @@ -241,7 +243,7 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) pendingPaddingTarget = nullptr; accumulatedPadding = 0; chapters = element_cast(std::move(element)); - if(!chapters->read(file)) + if(!chapters->read(file, elementCount)) return false; pendingPaddingTarget = chapters.get(); }