More robust checks for invalid MPEG frame headers.

diff --git a/taglib/mpeg/mpegheader.cpp b/taglib/mpeg/mpegheader.cpp
index 01cc6c57bd8d66ac2d1079f466506fe877b45338..05177f3e79791d7494556d985a8377b35b578eba 100644 (file)
--- a/taglib/mpeg/mpegheader.cpp
+++ b/taglib/mpeg/mpegheader.cpp
@@ -23,8 +23,6 @@
  *   http://www.mozilla.org/MPL/                                           *
  ***************************************************************************/
 
-#include <bitset>
-
 #include <tbytevector.h>
 #include <tstring.h>
 #include <tdebug.h>
@@ -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<unsigned char>(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<unsigned char>(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<unsigned char>(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<unsigned char>(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<unsigned char>(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<unsigned char>(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<unsigned char>(data[2]) >> 4;
+  const int bitrateIndex = (static_cast<unsigned char>(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<unsigned char>(data[2]) >> 2 & 0x03;
+  const int samplerateIndex = (static_cast<unsigned char>(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<ChannelMode>((static_cast<unsigned char>(data[3]) & 0xC0) >> 6);
+  d->channelMode = static_cast<ChannelMode>((static_cast<unsigned char>(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<unsigned char>(data[3]) & 0x04) != 0);
+  d->isCopyrighted = ((static_cast<unsigned char>(data[3]) & 0x08) != 0);
+  d->isPadded      = ((static_cast<unsigned char>(data[2]) & 0x02) != 0);
 
   // Samples per frame
 

diff --git a/taglib/mpeg/mpegproperties.cpp b/taglib/mpeg/mpegproperties.cpp
index 9fece404d42f3fb1b0041fcd67a17aaeacf35b91..989c9065e9a6cc41c61a5abf5f66808a472004bf 100644 (file)
--- 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 (file)
index 0000000..a6dc111
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 a4ceced425f8472745f5c3dcc0c26789d56be046..c04eecd9893219e5a3ee4d8a3f8e484183f2c0e1 100644 (file)
--- 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"));