Increment the version number

The bug report has a WAVE file with zero-sized 'data' chunk, which causes
TagLib to iterate over the file, 8 bytes in each iteration. The new code
adds a check for the chunk name, which forces it to mark the file as
invalid if the chunk name doesn't contain ASCII characters.

https://bugs.kde.org/show_bug.cgi?id=283412

CMakeLists.txt

@@ -40,7 +40,7 @@ endif (WIN32)
 
 SET(TAGLIB_LIB_MAJOR_VERSION "1")
 SET(TAGLIB_LIB_MINOR_VERSION "7")
-SET(TAGLIB_LIB_PATCH_VERSION "0")
+SET(TAGLIB_LIB_PATCH_VERSION "1")
 
 SET(TAGLIB_LIB_VERSION_STRING "${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION}")
 

NEWS

@@ -1,3 +1,13 @@
+TagLib 1.7.1 (Mar 17, 2012)
+===========================
+
+ * Improved parsing of corrupted WMA, RIFF and OGG files.
+ * Fixed a memory leak in the WMA parser.
+ * Fixed a memory leak in the FLAC parser.
+ * Fixed a possible division by zero in the APE parser.
+ * Added detection of TTA2 files.
+ * Fixed saving of multiple identically named tags to Vorbis Comments.
+
 TagLib 1.7 (Mar 11, 2011)
 =========================
 

taglib/ape/apeproperties.cpp

@@ -193,7 +193,7 @@ void APE::Properties::analyzeCurrent()
   uint blocksPerFrame = header.mid(4, 4).toUInt(false);
   uint finalFrameBlocks = header.mid(8, 4).toUInt(false);
   uint totalBlocks = totalFrames > 0 ? (totalFrames -  1) * blocksPerFrame + finalFrameBlocks : 0;
-  d->length = totalBlocks / d->sampleRate;
+  d->length = d->sampleRate > 0 ? totalBlocks / d->sampleRate : 0;
   d->bitrate = d->length > 0 ? ((d->streamLength * 8L) / d->length) / 1000 : 0;
 }
 

taglib/asf/asffile.cpp

@@ -141,11 +141,19 @@ class ASF::File::HeaderExtensionObject : public ASF::File::BaseObject
 {
 public:
   List<ASF::File::BaseObject *> objects;
+  ~HeaderExtensionObject();
   ByteVector guid();
   void parse(ASF::File *file, uint size);
   ByteVector render(ASF::File *file);
 };
 
+ASF::File::HeaderExtensionObject::~HeaderExtensionObject()
+{
+  for(unsigned int i = 0; i < objects.size(); i++) {
+    delete objects[i];
+  }
+}
+
 void ASF::File::BaseObject::parse(ASF::File *file, unsigned int size)
 {
   data.clear();

taglib/flac/flacfile.cpp

@@ -161,10 +161,12 @@ bool FLAC::File::save()
     MetadataBlock *block = d->blocks[i];
     if(block->code() == MetadataBlock::VorbisComment) {
       // Set the new Vorbis Comment block
+      delete block;
       block = new UnknownMetadataBlock(MetadataBlock::VorbisComment, d->xiphCommentData);
       foundVorbisCommentBlock = true;
     }
     if(block->code() == MetadataBlock::Padding) {
+      delete block;
       continue;
     }
     newBlocks.append(block);
@@ -190,7 +192,7 @@ bool FLAC::File::save()
   // Adjust the padding block(s)
 
   long originalLength = d->streamStart - d->flacStart;
-  int paddingLength = originalLength - data.size() - 4; 
+  int paddingLength = originalLength - data.size() - 4;
   if (paddingLength < 0) {
     paddingLength = MinPaddingLength;
   }

taglib/ogg/xiphcomment.cpp

@@ -287,7 +287,7 @@ void Ogg::XiphComment::parse(const ByteVector &data)
 
   int pos = 0;
 
-  int vendorLength = data.mid(0, 4).toUInt(false);
+  uint vendorLength = data.mid(0, 4).toUInt(false);
   pos += 4;
 
   d->vendorID = String(data.mid(pos, vendorLength), String::UTF8);
@@ -295,21 +295,31 @@ void Ogg::XiphComment::parse(const ByteVector &data)
 
   // Next the number of fields in the comment vector.
 
-  int commentFields = data.mid(pos, 4).toUInt(false);
+  uint commentFields = data.mid(pos, 4).toUInt(false);
   pos += 4;
 
-  for(int i = 0; i < commentFields; i++) {
+  if(commentFields > (data.size() - 8) / 4) {
+    return;
+  }
+
+  for(uint i = 0; i < commentFields; i++) {
 
     // Each comment field is in the format "KEY=value" in a UTF8 string and has
     // 4 bytes before the text starts that gives the length.
 
-    int commentLength = data.mid(pos, 4).toUInt(false);
+    uint commentLength = data.mid(pos, 4).toUInt(false);
     pos += 4;
 
     String comment = String(data.mid(pos, commentLength), String::UTF8);
     pos += commentLength;
+    if(pos > data.size()) {
+      break;
+    }
 
     int commentSeparatorPosition = comment.find("=");
+    if(commentSeparatorPosition == -1) {
+      break;
+    }
 
     String key = comment.substr(0, commentSeparatorPosition);
     String value = comment.substr(commentSeparatorPosition + 1);

taglib/riff/aiff/aifffile.cpp

@@ -87,6 +87,11 @@ bool RIFF::AIFF::File::save()
     return false;
   }
 
+  if(!isValid()) {
+    debug("RIFF::AIFF::File::save() -- Trying to save invalid file.");
+    return false;
+  }
+
   setChunkData(d->tagChunkID, d->tag->render());
 
   return true;

taglib/riff/aiff/aiffproperties.cpp

@@ -149,5 +149,5 @@ void RIFF::AIFF::Properties::read(const ByteVector &data)
   double sampleRate = ConvertFromIeeeExtended(reinterpret_cast<unsigned char *>(data.mid(8, 10).data()));
   d->sampleRate     = sampleRate;
   d->bitrate        = (sampleRate * d->sampleWidth * d->channels) / 1000.0;
-  d->length         = sampleFrames / d->sampleRate;
+  d->length         = d->sampleRate > 0 ? sampleFrames / d->sampleRate : 0;
 }

taglib/riff/rifffile.cpp

@@ -194,6 +194,19 @@ void RIFF::File::setChunkData(const ByteVector &name, const ByteVector &data)
 // private members
 ////////////////////////////////////////////////////////////////////////////////
 
+static bool isValidChunkID(const ByteVector &name)
+{
+  if(name.size() != 4) {
+    return false;
+  }
+  for(int i = 0; i < 4; i++) {
+    if(name[i] < 32 || name[i] > 127) {
+      return false;
+    }
+  }
+  return true;
+}
+
 void RIFF::File::read()
 {
   bool bigEndian = (d->endianness == BigEndian);
@@ -207,8 +220,15 @@ void RIFF::File::read()
     ByteVector chunkName = readBlock(4);
     uint chunkSize = readBlock(4).toUInt(bigEndian);
 
+    if(!isValidChunkID(chunkName)) {
+      debug("RIFF::File::read() -- Chunk '" + chunkName + "' has invalid ID");
+      setValid(false);
+      break;
+    }
+
     if(tell() + chunkSize > uint(length())) {
-      // something wrong
+      debug("RIFF::File::read() -- Chunk '" + chunkName + "' has invalid size (larger than the file size)");
+      setValid(false);
       break;
     }
 

taglib/riff/wav/wavfile.cpp

@@ -87,6 +87,11 @@ bool RIFF::WAV::File::save()
     return false;
   }
 
+  if(!isValid()) {
+    debug("RIFF::WAV::File::save() -- Trying to save invalid file.");
+    return false;
+  }
+
   setChunkData(d->tagChunkID, d->tag->render());
 
   return true;

taglib/toolkit/tbytevector.cpp

@@ -363,7 +363,7 @@ ByteVector ByteVector::mid(uint index, uint length) const
 
   ConstIterator endIt;
 
-  if(length < 0xffffffff && length + index < size())
+  if(length < size() - index)
     endIt = d->data.begin() + index + length;
   else
     endIt = d->data.end();

taglib/trueaudio/trueaudioproperties.cpp

@@ -118,19 +118,26 @@ void TrueAudio::Properties::read()
   int pos = 3;
 
   d->version = d->data[pos] - '0';
-  pos += 1 + 2;
+  pos += 1;
 
-  d->channels = d->data.mid(pos, 2).toShort(false);
-  pos += 2;
+  // According to http://en.true-audio.com/TTA_Lossless_Audio_Codec_-_Format_Description
+  // TTA2 headers are in development, and have a different format
+  if(1 == d->version) {
+    // Skip the audio format
+    pos += 2;
 
-  d->bitsPerSample = d->data.mid(pos, 2).toShort(false);
-  pos += 2;
+    d->channels = d->data.mid(pos, 2).toShort(false);
+    pos += 2;
 
-  d->sampleRate = d->data.mid(pos, 4).toUInt(false);
-  pos += 4;
+    d->bitsPerSample = d->data.mid(pos, 2).toShort(false);
+    pos += 2;
 
-  unsigned long samples = d->data.mid(pos, 4).toUInt(false);
-  d->length = samples / d->sampleRate;
+    d->sampleRate = d->data.mid(pos, 4).toUInt(false);
+    pos += 4;
 
-  d->bitrate = d->length > 0 ? ((d->streamLength * 8L) / d->length) / 1000 : 0;
+    uint sampleFrames = d->data.mid(pos, 4).toUInt(false);
+    d->length = d->sampleRate > 0 ? sampleFrames / d->sampleRate : 0;
+
+    d->bitrate = d->length > 0 ? ((d->streamLength * 8L) / d->length) / 1000 : 0;
+  }
 }

tests/test_wav.cpp

@@ -13,6 +13,7 @@ class TestWAV : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestWAV);
   CPPUNIT_TEST(testLength);
+  CPPUNIT_TEST(testZeroSizeDataChunk);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -20,9 +21,16 @@ public:
   void testLength()
   {
     RIFF::WAV::File f("data/empty.wav");
+    CPPUNIT_ASSERT_EQUAL(true, f.isValid());
     CPPUNIT_ASSERT_EQUAL(3, f.audioProperties()->length());
   }
 
+  void testZeroSizeDataChunk()
+  {
+    RIFF::WAV::File f("data/zero-size-chunk.wav");
+    CPPUNIT_ASSERT_EQUAL(false, f.isValid());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestWAV);