Merge pull request #496 from TsudaKageyu/fuzzed-mpc

Some fixes for fuzzed MPC files.

taglib/mpc/mpcproperties.cpp

@@ -155,75 +155,115 @@ int MPC::Properties::albumPeak() const
 // private members
 ////////////////////////////////////////////////////////////////////////////////
 
-unsigned long readSize(File *file, TagLib::uint &sizelength)
+unsigned long readSize(File *file, TagLib::uint &sizeLength, bool &eof)
 {
+  sizeLength = 0;
+  eof = false;
+
   unsigned char tmp;
   unsigned long size = 0;
 
   do {
-    ByteVector b = file->readBlock(1);
+    const ByteVector b = file->readBlock(1);
+    if(b.isEmpty()) {
+      eof = true;
+      break;
+    }
+
     tmp = b[0];
     size = (size << 7) | (tmp & 0x7F);
-    sizelength++;
+    sizeLength++;
   } while((tmp & 0x80));
   return size;
 }
 
-unsigned long readSize(const ByteVector &data, TagLib::uint &sizelength)
+unsigned long readSize(const ByteVector &data, TagLib::uint &pos)
 {
   unsigned char tmp;
   unsigned long size = 0;
-  unsigned long pos = 0;
 
   do {
     tmp = data[pos++];
     size = (size << 7) | (tmp & 0x7F);
-    sizelength++;
   } while((tmp & 0x80) && (pos < data.size()));
   return size;
 }
 
-static const unsigned short sftable [4] = { 44100, 48000, 37800, 32000 };
+// This array looks weird, but the same as original MusePack code found at:
+// https://www.musepack.net/index.php?pg=src
+static const unsigned short sftable [8] = { 44100, 48000, 37800, 32000, 0, 0, 0, 0 };
 
 void MPC::Properties::readSV8(File *file)
 {
   bool readSH = false, readRG = false;
 
   while(!readSH && !readRG) {
-    ByteVector packetType = file->readBlock(2);
-    uint packetSizeLength = 0;
-    unsigned long packetSize = readSize(file, packetSizeLength);
-    unsigned long dataSize = packetSize - 2 - packetSizeLength;
+    const ByteVector packetType = file->readBlock(2);
+
+    uint packetSizeLength;
+    bool eof;
+    const unsigned long packetSize = readSize(file, packetSizeLength, eof);
+    if(eof) {
+      debug("MPC::Properties::readSV8() - Reached to EOF.");
+      break;
+    }
+
+    const unsigned long dataSize = packetSize - 2 - packetSizeLength;
+
+    const ByteVector data = file->readBlock(dataSize);
+    if(data.size() != dataSize) {
+      debug("MPC::Properties::readSV8() - dataSize doesn't match the actual data size.");
+      break;
+    }
 
     if(packetType == "SH") {
       // Stream Header
       // http://trac.musepack.net/wiki/SV8Specification#StreamHeaderPacket
-      ByteVector data = file->readBlock(dataSize);
+
+      if(dataSize <= 5) {
+        debug("MPC::Properties::readSV8() - \"SH\" packet is too short to parse.");
+        break;
+      }
+
       readSH = true;
 
       TagLib::uint pos = 4;
       d->version = data[pos];
       pos += 1;
-      d->sampleFrames = readSize(data.mid(pos), pos);
-      ulong begSilence = readSize(data.mid(pos), pos);
+      d->sampleFrames = readSize(data, pos);
+      if(pos > dataSize - 3) {
+        debug("MPC::Properties::readSV8() - \"SH\" packet is corrupt.");
+        break;
+      }
 
-      std::bitset<16> flags(TAGLIB_CONSTRUCT_BITSET(data.toUShort(pos, true)));
+      ulong begSilence = readSize(data, pos);
+      if(pos > dataSize - 2) {
+        debug("MPC::Properties::readSV8() - \"SH\" packet is corrupt.");
+        break;
+      }
+
+      const ushort flags = data.toUShort(pos, true);
       pos += 2;
 
-      d->sampleRate = sftable[flags[15] * 4 + flags[14] * 2 + flags[13]];
-      d->channels = flags[7] * 8 + flags[6] * 4 + flags[5] * 2 + flags[4] + 1;
+      d->sampleRate = sftable[(flags >> 13) & 0x07];
+      d->channels   = ((flags >> 4) & 0x0F) + 1;
 
-      if((d->sampleFrames - begSilence) != 0)
-        d->bitrate = (int)(d->streamLength * 8.0 * d->sampleRate / (d->sampleFrames - begSilence));
-      d->bitrate = d->bitrate / 1000;
-
-      d->length = (d->sampleFrames - begSilence) / d->sampleRate;
+      const uint frameCount = d->sampleFrames - begSilence;
+      if(frameCount != 0 && d->sampleRate != 0) {
+        d->bitrate = (int)(d->streamLength * 8.0 * d->sampleRate / frameCount / 1000);
+        d->length  = frameCount / d->sampleRate;
+      }
     }
 
     else if (packetType == "RG") {
       // Replay Gain
       // http://trac.musepack.net/wiki/SV8Specification#ReplaygainPacket
-      ByteVector data = file->readBlock(dataSize);
+
+      if(dataSize <= 9) {
+        debug("MPC::Properties::readSV8() - \"RG\" packet is too short to parse.");
+        break;
+      }
+
       readRG = true;
 
       int replayGainVersion = data[0];

tests/data/segfault2.mpc

@@ -0,0 +1 @@
+MPCKSH

tests/test_mpc.cpp

@@ -1,10 +1,10 @@
-#include <cppunit/extensions/HelperMacros.h>
 #include <string>
 #include <stdio.h>
 #include <tag.h>
 #include <tstringlist.h>
 #include <tbytevectorlist.h>
 #include <mpcfile.h>
+#include <cppunit/extensions/HelperMacros.h>
 #include "utils.h"
 
 using namespace std;
@@ -17,6 +17,10 @@ class TestMPC : public CppUnit::TestFixture
   CPPUNIT_TEST(testPropertiesSV7);
   CPPUNIT_TEST(testPropertiesSV5);
   CPPUNIT_TEST(testPropertiesSV4);
+  CPPUNIT_TEST(testFuzzedFile1);
+  CPPUNIT_TEST(testFuzzedFile2);
+  CPPUNIT_TEST(testFuzzedFile3);
+  CPPUNIT_TEST(testFuzzedFile4);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -61,6 +65,30 @@ public:
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
   }
 
+  void testFuzzedFile1()
+  {
+    MPC::File f(TEST_FILE_PATH_C("zerodiv.mpc"));
+    CPPUNIT_ASSERT(f.isValid());
+  }
+
+  void testFuzzedFile2()
+  {
+    MPC::File f(TEST_FILE_PATH_C("infloop.mpc"));
+    CPPUNIT_ASSERT(f.isValid());
+  }
+
+  void testFuzzedFile3()
+  {
+    MPC::File f(TEST_FILE_PATH_C("segfault.mpc"));
+    CPPUNIT_ASSERT(f.isValid());
+  }
+
+  void testFuzzedFile4()
+  {
+    MPC::File f(TEST_FILE_PATH_C("segfault2.mpc"));
+    CPPUNIT_ASSERT(f.isValid());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPC);