diff --git a/taglib/mpc/mpcproperties.cpp b/taglib/mpc/mpcproperties.cpp index d406f8d7..128eff42 100644 --- a/taglib/mpc/mpcproperties.cpp +++ b/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]; diff --git a/tests/data/infloop.mpc b/tests/data/infloop.mpc new file mode 100644 index 00000000..46861ab3 Binary files /dev/null and b/tests/data/infloop.mpc differ diff --git a/tests/data/segfault.mpc b/tests/data/segfault.mpc new file mode 100644 index 00000000..2c7e29fb Binary files /dev/null and b/tests/data/segfault.mpc differ diff --git a/tests/data/segfault2.mpc b/tests/data/segfault2.mpc new file mode 100644 index 00000000..fcfa982f --- /dev/null +++ b/tests/data/segfault2.mpc @@ -0,0 +1 @@ +MPCKSH \ No newline at end of file diff --git a/tests/data/zerodiv.mpc b/tests/data/zerodiv.mpc new file mode 100644 index 00000000..d3ea57c7 Binary files /dev/null and b/tests/data/zerodiv.mpc differ diff --git a/tests/test_mpc.cpp b/tests/test_mpc.cpp index 12da6ed3..0589740f 100644 --- a/tests/test_mpc.cpp +++ b/tests/test_mpc.cpp @@ -1,10 +1,10 @@ -#include #include #include #include #include #include #include +#include #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);