From eb698de6e5ca7904a481ba92cc143da281cd49df Mon Sep 17 00:00:00 2001 From: Anthony Date: Sat, 11 Jul 2026 09:02:39 +0200 Subject: [PATCH] [Matroska] Fix unknown size elements (#1377) In matroska an element data size of a VINT with all bits 1 means the data size is unknown. Unknown data size can only apply to Master Elements. Unknown sized elements are described in https://datatracker.ietf.org/doc/rfc8794/ section 6.2 It gives the following 5 conditions for detecting the end of an unknown sized element: * Any EBML Element that is a valid Parent Element of the Unknown- Sized Element according to the EBML Schema, Global Elements excluded. * Any valid EBML Element according to the EBML Schema, Global Elements excluded, that is not a Descendant Element of the Unknown-Sized Element but shares a common direct parent, such as a Top-Level Element. * Any EBML Element that is a valid Root Element according to the EBML Schema, Global Elements excluded. * The end of the Parent Element with a known size has been reached. * The end of the EBML Document, either when reaching the end of the file or because a new EBML Header started. In this patch we use the higher level maxOffset to determine the maximum data size for the element, which matches the fourth condition, but is incomplete without the other four methods. As only Segment and Cluster elements of Matroska files are allowed to use unknown size length and TagLib does not process Cluster elements, this should be sufficient. --------- Signed-off-by: Anthony Brandon Co-authored-by: Urs Fleisch --- taglib/matroska/ebml/ebmlelement.cpp | 6 ++-- taglib/matroska/ebml/ebmlutils.h | 12 +++++++ tests/test_matroska.cpp | 51 ++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/taglib/matroska/ebml/ebmlelement.cpp b/taglib/matroska/ebml/ebmlelement.cpp index bde73ff6..27b43240 100644 --- a/taglib/matroska/ebml/ebmlelement.cpp +++ b/taglib/matroska/ebml/ebmlelement.cpp @@ -52,12 +52,14 @@ std::unique_ptr EBML::Element::factory(File &file, offset_t maxOf } // Get the size length and data length - const auto &[sizeLength, dataSize] = readVINT(file); + auto [sizeLength, dataSize] = readVINT(file); if(!sizeLength) return nullptr; if(const offset_t currentOffset = file.tell(); - static_cast(dataSize) > maxOffset - currentOffset) { + isUnknownSize(sizeLength, dataSize)) { + dataSize = maxOffset - currentOffset; + } else if(static_cast(dataSize) > maxOffset - currentOffset) { debug(Utils::formatString("EBML: datasize too great: %lu > (%lld - %lld)", dataSize, maxOffset, currentOffset)); return nullptr; diff --git a/taglib/matroska/ebml/ebmlutils.h b/taglib/matroska/ebml/ebmlutils.h index 4038e5b0..f35228dd 100644 --- a/taglib/matroska/ebml/ebmlutils.h +++ b/taglib/matroska/ebml/ebmlutils.h @@ -71,6 +71,18 @@ namespace TagLib { return 3; return 4; } + + // See https://datatracker.ietf.org/doc/rfc8794/ section 6.2 + constexpr bool isUnknownSize(unsigned int sizeLength, uint64_t dataSize) + { + if(sizeLength == 0 || sizeLength > 8) { + return false; + } + + const unsigned int numDataBits = sizeLength * 8 - sizeLength; + const uint64_t mask = (1ULL << numDataBits) - 1; + return (dataSize & mask) == mask; + } } } diff --git a/tests/test_matroska.cpp b/tests/test_matroska.cpp index 2e2746e7..5ec0e79e 100644 --- a/tests/test_matroska.cpp +++ b/tests/test_matroska.cpp @@ -160,6 +160,7 @@ class TestMatroska : public CppUnit::TestFixture CPPUNIT_TEST(testSaveTypesBeforeCues); CPPUNIT_TEST(testSaveTypesNoTrailingVoid); CPPUNIT_TEST(testSaveTypesReclaimVoid); + CPPUNIT_TEST(testUnknownSizeSegment); CPPUNIT_TEST_SUITE_END(); public: @@ -1777,6 +1778,56 @@ public: } } + void testUnknownSizeSegment() + { + ScopedFileCopy copy("no-tags", ".mka"); + string newname = copy.fileName(); + + // Modify the copied file (/tmp/taglib-test.mka on Linux) to have elements + // with unknown size length. + { + PlainFile file(newname.c_str()); + ByteVector fileData = file.readAll(); + CPPUNIT_ASSERT_EQUAL(12390U, fileData.size()); + // Size of Segment + for(int i = 0x2d; i <= 0x33; ++i) { + fileData[i] = '\xff'; + } + // Size of Cluster + for(int i = 0x2d; i <= 0x33; ++i) { + fileData[i] = '\xff'; + } + fileData[0x1482] = '\x7f'; + fileData[0x1483] = '\xff'; + file.seek(0); + file.writeBlock(fileData); + } + { + Matroska::File f(newname.c_str(), true, AudioProperties::Accurate); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(f.audioProperties()); + CPPUNIT_ASSERT_EQUAL(444, f.audioProperties()->lengthInMilliseconds()); + CPPUNIT_ASSERT_EQUAL(223, f.audioProperties()->bitrate()); + CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); + CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); + CPPUNIT_ASSERT_EQUAL(String("matroska"), f.audioProperties()->docType()); + CPPUNIT_ASSERT_EQUAL(4, f.audioProperties()->docTypeVersion()); + CPPUNIT_ASSERT_EQUAL(String("A_MPEG/L3"), f.audioProperties()->codecName()); + CPPUNIT_ASSERT_EQUAL(String(""), f.audioProperties()->title()); + CPPUNIT_ASSERT(!f.tag(false)); + CPPUNIT_ASSERT(!f.attachments(false)); + auto tag = f.tag(true); + CPPUNIT_ASSERT(tag->isEmpty()); + tag->setTitle("Unknown size"); + CPPUNIT_ASSERT(f.save()); + } + { + Matroska::File f(newname.c_str(), true, AudioProperties::Accurate); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(f.tag(false)); + CPPUNIT_ASSERT_EQUAL(String("Unknown size"), f.tag()->title()); + } + } }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMatroska);