Fix an out-of-bounds access and consequent errors while parsing fuzzed MPC files.

Consequent errors may vary: segfault, zerodiv and so forth.

diff --git a/taglib/mpc/mpcproperties.cpp b/taglib/mpc/mpcproperties.cpp
index d406f8d7356b9caf2cdbed8386e0bb1a8a0b356c..f11f8ecf7a5bfcba642b646a710647bd307e2214 100644 (file)
--- a/taglib/mpc/mpcproperties.cpp
+++ b/taglib/mpc/mpcproperties.cpp
@@ -183,7 +183,9 @@ unsigned long readSize(const ByteVector &data, TagLib::uint &sizelength)
   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)
 {
@@ -207,17 +209,17 @@ void MPC::Properties::readSV8(File *file)
       d->sampleFrames = readSize(data.mid(pos), pos);
       ulong begSilence = readSize(data.mid(pos), pos);
 
-      std::bitset<16> flags(TAGLIB_CONSTRUCT_BITSET(data.toUShort(pos, true)));
+      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") {

diff --git a/tests/data/zerodiv.mpc b/tests/data/zerodiv.mpc
new file mode 100644 (file)
index 0000000..d3ea57c
Binary files /dev/null and b/tests/data/zerodiv.mpc differ

diff --git a/tests/test_mpc.cpp b/tests/test_mpc.cpp
index 12da6ed3feb4fd6ec140cf0ac3ebe468eb1a192f..d7239a6dcc92f3b3b68403dc46951073e67c7b76 100644 (file)
--- a/tests/test_mpc.cpp
+++ b/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,7 @@ class TestMPC : public CppUnit::TestFixture
   CPPUNIT_TEST(testPropertiesSV7);
   CPPUNIT_TEST(testPropertiesSV5);
   CPPUNIT_TEST(testPropertiesSV4);
+  CPPUNIT_TEST(testFuzzedFile1);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -61,6 +62,12 @@ public:
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
   }
 
+  void testFuzzedFile1()
+  {
+    MPC::File f(TEST_FILE_PATH_C("zerodiv.mpc"));
+    CPPUNIT_ASSERT(f.isValid());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPC);