From 13239fde268c891f066e6cf4930896075aeaad2a Mon Sep 17 00:00:00 2001 From: Anthony Brandon Date: Wed, 24 Jun 2026 14:03:06 +0200 Subject: [PATCH] [Matroska] Check element length before constructing By checking the Element size against the maximum offset we can find out of bounds elements early and not try to read them at all. This is useful for the next patch. Signed-off-by: Anthony Brandon --- taglib/matroska/ebml/ebmlelement.cpp | 9 ++++++++- taglib/matroska/ebml/ebmlelement.h | 2 +- taglib/matroska/ebml/ebmlmksegment.cpp | 2 +- taglib/matroska/ebml/ebmlutils.cpp | 4 ++-- taglib/matroska/matroskafile.cpp | 4 ++-- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/taglib/matroska/ebml/ebmlelement.cpp b/taglib/matroska/ebml/ebmlelement.cpp index c384e3d5..bde73ff6 100644 --- a/taglib/matroska/ebml/ebmlelement.cpp +++ b/taglib/matroska/ebml/ebmlelement.cpp @@ -41,7 +41,7 @@ using namespace TagLib; #define RETURN_ELEMENT_FOR_CASE(eid) \ case (eid): return make_unique_element(id, sizeLength, dataSize, offset) -std::unique_ptr EBML::Element::factory(File &file) +std::unique_ptr EBML::Element::factory(File &file, offset_t maxOffset) { // Get the element ID const offset_t offset = file.tell(); @@ -56,6 +56,13 @@ std::unique_ptr EBML::Element::factory(File &file) if(!sizeLength) return nullptr; + if(const offset_t currentOffset = file.tell(); + static_cast(dataSize) > maxOffset - currentOffset) { + debug(Utils::formatString("EBML: datasize too great: %lu > (%lld - %lld)", + dataSize, maxOffset, currentOffset)); + return nullptr; + } + // Return the subclass // The enum switch without default will give us a warning if an ID is missing auto id = static_cast(uintId); diff --git a/taglib/matroska/ebml/ebmlelement.h b/taglib/matroska/ebml/ebmlelement.h index 19c6a92d..e6c4ded3 100644 --- a/taglib/matroska/ebml/ebmlelement.h +++ b/taglib/matroska/ebml/ebmlelement.h @@ -122,7 +122,7 @@ namespace TagLib int64_t getDataSize() const; ByteVector renderId() const; virtual ByteVector render(); - static std::unique_ptr factory(File &file); + static std::unique_ptr factory(File &file, offset_t maxOffset); static unsigned int readId(File &file); protected: diff --git a/taglib/matroska/ebml/ebmlmksegment.cpp b/taglib/matroska/ebml/ebmlmksegment.cpp index b9392938..88fcdd4c 100644 --- a/taglib/matroska/ebml/ebmlmksegment.cpp +++ b/taglib/matroska/ebml/ebmlmksegment.cpp @@ -45,7 +45,7 @@ std::unique_ptr readElementAt(File &file, } file.seek(offset); - auto element = EBML::Element::factory(file); + auto element = EBML::Element::factory(file, maxOffset); if(!element || element->getId() != Id) { return nullptr; } diff --git a/taglib/matroska/ebml/ebmlutils.cpp b/taglib/matroska/ebml/ebmlutils.cpp index 7c37f00d..b76cdf2c 100644 --- a/taglib/matroska/ebml/ebmlutils.cpp +++ b/taglib/matroska/ebml/ebmlutils.cpp @@ -34,7 +34,7 @@ std::unique_ptr EBML::findElement(File &file, Element::Id id, off { std::unique_ptr element; while(file.tell() < maxOffset) { - element = Element::factory(file); + element = Element::factory(file, maxOffset); if(!element || element->getId() == id) return element; element->skipData(file); @@ -45,7 +45,7 @@ std::unique_ptr EBML::findElement(File &file, Element::Id id, off std::unique_ptr EBML::findNextElement(File &file, offset_t maxOffset) { - return file.tell() < maxOffset ? Element::factory(file) : nullptr; + return file.tell() < maxOffset ? Element::factory(file, maxOffset) : nullptr; } template diff --git a/taglib/matroska/matroskafile.cpp b/taglib/matroska/matroskafile.cpp index 627f0810..7ff001bd 100644 --- a/taglib/matroska/matroskafile.cpp +++ b/taglib/matroska/matroskafile.cpp @@ -368,7 +368,7 @@ void Matroska::File::read(bool readProperties, Properties::ReadStyle readStyle) // Find the EBML Header const auto head = EBML::element_cast( - EBML::Element::factory(*this)); + EBML::Element::factory(*this, fileLength)); if(!head || head->getId() != EBML::Element::Id::EBMLHeader) { debug("Failed to find EBML head"); setValid(false); @@ -381,7 +381,7 @@ void Matroska::File::read(bool readProperties, Properties::ReadStyle readStyle) head->skipData(*this); } - offset_t maxOffset = fileLength - tell(); + offset_t maxOffset = fileLength; if (readStyle == Properties::ReadStyle::Fast && maxOffset > FAST_SCAN_LIMIT) { maxOffset = FAST_SCAN_LIMIT; }