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

@@ -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");

taglib/ogg/oggfile.cpp

@@ -25,6 +25,7 @@
 
 #include "oggfile.h"
 
+#include <limits>
 #include <utility>
 
 #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<unsigned int>::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<unsigned int>::max());
+}
+
+bool Ogg::File::readPages(unsigned int i, unsigned int maxSize)
+{
+  const bool limitPacketSize = maxSize != std::numeric_limits<unsigned int>::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);
   }
 }

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.
        */