From 77f547760995217dd5fa321b162fa9149272989d Mon Sep 17 00:00:00 2001 From: Acts1631 <69813585+acts-1631@users.noreply.github.com> Date: Thu, 13 Aug 2026 00:40:24 -0400 Subject: [PATCH] Ogg FLAC: limit metadata packet size (#1407) Reject metadata packets larger than FLAC's 24-bit payload limit. Continued Ogg pages could otherwise make TagLib allocate arbitrary amounts of memory before validating the FLAC block length. Add bounded Ogg packet reassembly for Ogg FLAC metadata parsing. --- taglib/ogg/flac/oggflacfile.cpp | 11 ++++-- taglib/ogg/oggfile.cpp | 60 +++++++++++++++++++++++++++++++-- taglib/ogg/oggfile.h | 12 +++++++ 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/taglib/ogg/flac/oggflacfile.cpp b/taglib/ogg/flac/oggflacfile.cpp index a71dc72b..b428bf72 100644 --- a/taglib/ogg/flac/oggflacfile.cpp +++ b/taglib/ogg/flac/oggflacfile.cpp @@ -34,6 +34,10 @@ using TagLib::FLAC::Properties; namespace { constexpr int MAX_OGG_FLAC_METADATA_BLOCK_COUNT = 1024; + // FLAC metadata payloads use a 24-bit length. The first Ogg FLAC packet + // also contains 13 mapping bytes and a 4-byte FLAC metadata header. + constexpr unsigned int MAX_OGG_FLAC_METADATA_PACKET_SIZE = + (1U << 24) + 17; } class Ogg::FLAC::File::FilePrivate @@ -230,7 +234,8 @@ void Ogg::FLAC::File::scan() int blockCount = 1; offset_t overhead = 0; - ByteVector metadataHeader = packet(ipacket); + ByteVector metadataHeader = + packet(ipacket, MAX_OGG_FLAC_METADATA_PACKET_SIZE); if(metadataHeader.isEmpty()) return; @@ -256,7 +261,7 @@ void Ogg::FLAC::File::scan() } else { // FLAC 1.1.0 & 1.1.1 - metadataHeader = packet(++ipacket); + metadataHeader = packet(++ipacket, MAX_OGG_FLAC_METADATA_PACKET_SIZE); } ByteVector header = metadataHeader.mid(0, 4); @@ -296,7 +301,7 @@ void Ogg::FLAC::File::scan() debug("Ogg::FLAC::File::scan() -- Maximum metadata block count exceeded"); return; } - metadataHeader = packet(++ipacket); + metadataHeader = packet(++ipacket, MAX_OGG_FLAC_METADATA_PACKET_SIZE); header = metadataHeader.mid(0, 4); if(header.size() != 4) { debug("Ogg::FLAC::File::scan() -- Invalid Ogg/FLAC metadata header"); diff --git a/taglib/ogg/oggfile.cpp b/taglib/ogg/oggfile.cpp index 47db66c8..f195a785 100644 --- a/taglib/ogg/oggfile.cpp +++ b/taglib/ogg/oggfile.cpp @@ -25,6 +25,7 @@ #include "oggfile.h" +#include #include #include "tdebug.h" @@ -76,17 +77,27 @@ public: Ogg::File::~File() = default; ByteVector Ogg::File::packet(unsigned int i) +{ + return packet(i, std::numeric_limits::max()); +} + +ByteVector Ogg::File::packet(unsigned int i, unsigned int maxSize) { // Check to see if we're called setPacket() for this packet since the last // save: - if(d->dirtyPackets.contains(i)) + if(d->dirtyPackets.contains(i)) { + if(d->dirtyPackets[i].size() > maxSize) { + debug("Ogg::File::packet() -- Maximum packet size exceeded"); + return ByteVector(); + } return d->dirtyPackets[i]; + } // If we haven't indexed the page where the packet we're interested in starts, // begin reading pages until we have. - if(!readPages(i)) { + if(!readPages(i, maxSize)) { debug("Ogg::File::packet() -- Could not find the requested packet."); return ByteVector(); } @@ -108,7 +119,12 @@ ByteVector Ogg::File::packet(unsigned int i) while(nextPacketIndex(*it) <= i) { ++it; - packet.append((*it)->packets().front()); + const ByteVector packetPart = (*it)->packets().front(); + if(packetPart.size() > maxSize - packet.size()) { + debug("Ogg::File::packet() -- Maximum packet size exceeded"); + return ByteVector(); + } + packet.append(packetPart); } return packet; @@ -219,6 +235,39 @@ bool Ogg::File::selectStream(const ByteVector &magic) bool Ogg::File::readPages(unsigned int i) { + return readPages(i, std::numeric_limits::max()); +} + +bool Ogg::File::readPages(unsigned int i, unsigned int maxSize) +{ + const bool limitPacketSize = maxSize != std::numeric_limits::max(); + unsigned int packetSize = 0; + const auto addPacketPartSize = [&](const Page *page) { + if(page->containsPacket(i) == Page::DoesNotContainPacket) + return true; + + const ByteVectorList packets = page->packets(); + const unsigned int packetPartIndex = i - page->firstPacketIndex(); + if(packetPartIndex >= packets.size()) + return false; + + const unsigned int packetPartSize = packets[packetPartIndex].size(); + if(packetPartSize > maxSize - packetSize) + return false; + + packetSize += packetPartSize; + return true; + }; + + if(limitPacketSize) { + for(const auto &page : std::as_const(d->pages)) { + if(!addPacketPartSize(page)) { + debug("Ogg::File::readPages() -- Maximum packet size exceeded"); + return false; + } + } + } + while(true) { // If we've already indexed the page containing packet i, we're done. @@ -268,6 +317,11 @@ bool Ogg::File::readPages(unsigned int i) = d->pages.isEmpty() ? 0 : nextPacketIndex(d->pages.back()); nextPage->setFirstPacketIndex(packetIndex); + if(limitPacketSize && !addPacketPartSize(nextPage)) { + debug("Ogg::File::readPages() -- Maximum packet size exceeded"); + delete nextPage; + return false; + } d->pages.append(nextPage); } } diff --git a/taglib/ogg/oggfile.h b/taglib/ogg/oggfile.h index 3a4849fa..3affda93 100644 --- a/taglib/ogg/oggfile.h +++ b/taglib/ogg/oggfile.h @@ -84,6 +84,12 @@ namespace TagLib { bool save() override; protected: + /*! + * Returns the packet contents for the i-th packet if its size does not + * exceed the requested maximum. + */ + ByteVector packet(unsigned int i, unsigned int maxSize); + /*! * Constructs an Ogg file from \a file. * @@ -126,6 +132,12 @@ namespace TagLib { */ bool readPages(unsigned int i); + /*! + * Reads the pages needed to compose the requested packet while limiting + * its total size. + */ + bool readPages(unsigned int i, unsigned int maxSize); + /*! * Writes the requested packet to the file. */