From: Tsuda Kageyu Date: Tue, 8 Dec 2015 02:11:50 +0000 (+0900) Subject: More robust checks for invalid MPEG frame headers. X-Git-Tag: v1.11beta~47^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=be9b5cc93a3535023fc2bacc1f3dda3364c83bf0;p=taglib More robust checks for invalid MPEG frame headers. --- diff --git a/taglib/mpeg/mpegheader.cpp b/taglib/mpeg/mpegheader.cpp index 01cc6c57..05177f3e 100644 --- a/taglib/mpeg/mpegheader.cpp +++ b/taglib/mpeg/mpegheader.cpp @@ -23,8 +23,6 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ -#include - #include #include #include @@ -68,20 +66,21 @@ public: // public members //////////////////////////////////////////////////////////////////////////////// -MPEG::Header::Header(const ByteVector &data) +MPEG::Header::Header(const ByteVector &data) : + d(new HeaderPrivate()) { - d = new HeaderPrivate; parse(data); } -MPEG::Header::Header(const Header &h) : d(h.d) +MPEG::Header::Header(const Header &h) : + d(h.d) { d->ref(); } MPEG::Header::~Header() { - if (d->deref()) + if(d->deref()) delete d; } @@ -164,39 +163,54 @@ MPEG::Header &MPEG::Header::operator=(const Header &h) void MPEG::Header::parse(const ByteVector &data) { - if(data.size() < 4 || static_cast(data[0]) != 0xff) { - debug("MPEG::Header::parse() -- First byte did not match MPEG synch."); + if(data.size() < 4) { + debug("MPEG::Header::parse() -- data is too short for an MPEG frame header."); return; } - std::bitset<32> flags(TAGLIB_CONSTRUCT_BITSET(data.toUInt())); + // Check for the MPEG synch bytes. - // Check for the second byte's part of the MPEG synch + if(static_cast(data[0]) != 0xFF) { + debug("MPEG::Header::parse() -- First byte did not match MPEG synch."); + return; + } - if(!flags[23] || !flags[22] || !flags[21]) { + if((static_cast(data[1]) & 0xE0) != 0xE0) { debug("MPEG::Header::parse() -- Second byte did not match MPEG synch."); return; } // Set the MPEG version - if(!flags[20] && !flags[19]) + const int versionBits = (static_cast(data[1]) >> 3) & 0x03; + + if(versionBits == 0) d->version = Version2_5; - else if(flags[20] && !flags[19]) + else if(versionBits == 2) d->version = Version2; - else if(flags[20] && flags[19]) + else if(versionBits == 3) d->version = Version1; + else { + debug("MPEG::Header::parse() -- Invalid MPEG version bits."); + return; + } // Set the MPEG layer - if(!flags[18] && flags[17]) + const int layerBits = (static_cast(data[1]) >> 1) & 0x03; + + if(layerBits == 1) d->layer = 3; - else if(flags[18] && !flags[17]) + else if(layerBits == 2) d->layer = 2; - else if(flags[18] && flags[17]) + else if(layerBits == 3) d->layer = 1; + else { + debug("MPEG::Header::parse() -- Invalid MPEG layer bits."); + return; + } - d->protectionEnabled = !flags[16]; + d->protectionEnabled = (static_cast(data[1] & 0x01) == 0); // Set the bitrate @@ -219,9 +233,14 @@ void MPEG::Header::parse(const ByteVector &data) // The bitrate index is encoded as the first 4 bits of the 3rd byte, // i.e. 1111xxxx - int i = static_cast(data[2]) >> 4; + const int bitrateIndex = (static_cast(data[2]) >> 4) & 0x0F; - d->bitrate = bitrates[versionIndex][layerIndex][i]; + d->bitrate = bitrates[versionIndex][layerIndex][bitrateIndex]; + + if(d->bitrate == 0) { + debug("MPEG::Header::parse() -- Invalid bit rate."); + return; + } // Set the sample rate @@ -233,9 +252,9 @@ void MPEG::Header::parse(const ByteVector &data) // The sample rate index is encoded as two bits in the 3nd byte, i.e. xxxx11xx - i = static_cast(data[2]) >> 2 & 0x03; + const int samplerateIndex = (static_cast(data[2]) >> 2) & 0x03; - d->sampleRate = sampleRates[d->version][i]; + d->sampleRate = sampleRates[d->version][samplerateIndex]; if(d->sampleRate == 0) { debug("MPEG::Header::parse() -- Invalid sample rate."); @@ -245,13 +264,13 @@ void MPEG::Header::parse(const ByteVector &data) // The channel mode is encoded as a 2 bit value at the end of the 3nd byte, // i.e. xxxxxx11 - d->channelMode = static_cast((static_cast(data[3]) & 0xC0) >> 6); + d->channelMode = static_cast((static_cast(data[3]) >> 6) & 0x03); // TODO: Add mode extension for completeness - d->isOriginal = flags[2]; - d->isCopyrighted = flags[3]; - d->isPadded = flags[9]; + d->isOriginal = ((static_cast(data[3]) & 0x04) != 0); + d->isCopyrighted = ((static_cast(data[3]) & 0x08) != 0); + d->isPadded = ((static_cast(data[2]) & 0x02) != 0); // Samples per frame diff --git a/taglib/mpeg/mpegproperties.cpp b/taglib/mpeg/mpegproperties.cpp index 9fece404..989c9065 100644 --- a/taglib/mpeg/mpegproperties.cpp +++ b/taglib/mpeg/mpegproperties.cpp @@ -155,27 +155,33 @@ bool MPEG::Properties::isOriginal() const void MPEG::Properties::read(File *file) { - // Only the first frame is required if we have a VBR header. + // Only the first valid frame is required if we have a VBR header. - const long first = file->firstFrameOffset(); - if(first < 0) { - debug("MPEG::Properties::read() -- Could not find a valid first MPEG frame in the stream."); + long firstFrameOffset = file->firstFrameOffset(); + if(firstFrameOffset < 0) { + debug("MPEG::Properties::read() -- Could not find an MPEG frame in the stream."); return; } - file->seek(first); - const Header firstHeader(file->readBlock(4)); + file->seek(firstFrameOffset); + Header firstHeader(file->readBlock(4)); - if(!firstHeader.isValid()) { - debug("MPEG::Properties::read() -- The first page header is invalid."); - return; + while(!firstHeader.isValid()) { + firstFrameOffset = file->nextFrameOffset(firstFrameOffset + 1); + if(firstFrameOffset < 0) { + debug("MPEG::Properties::read() -- Could not find a valid first MPEG frame in the stream."); + return; + } + + file->seek(firstFrameOffset); + firstHeader = Header(file->readBlock(4)); } // Check for a VBR header that will help us in gathering information about a // VBR stream. - file->seek(first + 4); - d->xingHeader = new XingHeader(file->readBlock(firstHeader.frameLength() - 4)); + file->seek(firstFrameOffset); + d->xingHeader = new XingHeader(file->readBlock(firstHeader.frameLength())); if(!d->xingHeader->isValid()) { delete d->xingHeader; d->xingHeader = 0; @@ -201,7 +207,7 @@ void MPEG::Properties::read(File *file) d->bitrate = firstHeader.bitrate(); - long streamLength = file->length() - first; + long streamLength = file->length() - firstFrameOffset; if(file->hasID3v1Tag()) streamLength -= 128; diff --git a/tests/data/invalid-frames.mp3 b/tests/data/invalid-frames.mp3 new file mode 100644 index 00000000..a6dc1117 Binary files /dev/null and b/tests/data/invalid-frames.mp3 differ diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index a4ceced4..c04eecd9 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -22,6 +22,7 @@ class TestMPEG : public CppUnit::TestFixture CPPUNIT_TEST(testAudioPropertiesXingHeaderVBR); CPPUNIT_TEST(testAudioPropertiesVBRIHeader); CPPUNIT_TEST(testAudioPropertiesNoVBRHeaders); + CPPUNIT_TEST(testSkipInvalidFrames); CPPUNIT_TEST(testVersion2DurationWithXingHeader); CPPUNIT_TEST(testSaveID3v24); CPPUNIT_TEST(testSaveID3v24WrongParam); @@ -102,6 +103,19 @@ public: CPPUNIT_ASSERT_EQUAL(209, lastHeader.frameLength()); } + void testSkipInvalidFrames() + { + MPEG::File f(TEST_FILE_PATH_C("invalid-frames.mp3")); + CPPUNIT_ASSERT(f.audioProperties()); + CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->length()); + CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->lengthInSeconds()); + CPPUNIT_ASSERT_EQUAL(393, f.audioProperties()->lengthInMilliseconds()); + CPPUNIT_ASSERT_EQUAL(160, f.audioProperties()->bitrate()); + CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); + CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); + CPPUNIT_ASSERT(!f.audioProperties()->xingHeader()); + } + void testVersion2DurationWithXingHeader() { MPEG::File f(TEST_FILE_PATH_C("mpeg2.mp3"));