Prepare 1.7.2 release

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 "2")
 
 SET(TAGLIB_LIB_VERSION_STRING "${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION}")
 
@@ -76,6 +76,7 @@ endif(NOT WIN32)
 
 INSTALL( PROGRAMS  ${CMAKE_CURRENT_BINARY_DIR}/taglib-config DESTINATION ${BIN_INSTALL_DIR})
 
-CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.cmake ${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile)
+CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.cmake ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile)
+file(COPY doc/taglib.png DESTINATION doc)
 ADD_CUSTOM_TARGET(docs doxygen)
 

Doxyfile.cmake

@@ -61,7 +61,7 @@ WARN_LOGFILE           =
 #---------------------------------------------------------------------------
 # configuration options related to the input files
 #---------------------------------------------------------------------------
-INPUT                  = taglib
+INPUT                  = @CMAKE_SOURCE_DIR@/taglib
 FILE_PATTERNS          = *.h \
                          *.hh \
                          *.H
@@ -96,9 +96,9 @@ IGNORE_PREFIX          =
 GENERATE_HTML          = YES
 HTML_OUTPUT            = html
 HTML_FILE_EXTENSION    = .html
-HTML_HEADER            = doc/api-header.html
-HTML_FOOTER            = doc/api-footer.html
-HTML_STYLESHEET        = doc/taglib-api.css
+HTML_HEADER            = @CMAKE_SOURCE_DIR@/doc/api-header.html
+HTML_FOOTER            = @CMAKE_SOURCE_DIR@/doc/api-footer.html
+HTML_STYLESHEET        = @CMAKE_SOURCE_DIR@/doc/taglib-api.css
 HTML_ALIGN_MEMBERS     = YES
 GENERATE_HTMLHELP      = NO
 CHM_FILE               = 

NEWS

@@ -1,3 +1,19 @@
+TagLib 1.7.2 (Apr 20, 2012)
+===========================
+
+ * Fixed division by zero while parsing corrupted MP4 files (CVE-2012-2396).
+ * Fixed compilation on Haiku.
+
+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/apefooter.cpp

@@ -24,7 +24,7 @@
  *   http://www.mozilla.org/MPL/                                           *
  ***************************************************************************/
 
-#include <ostream>
+#include <iostream>
 #include <bitset>
 
 #include <tstring.h>

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();
@@ -319,7 +327,16 @@ void ASF::File::HeaderExtensionObject::parse(ASF::File *file, uint /*size*/)
   long long dataPos = 0;
   while(dataPos < dataSize) {
     ByteVector guid = file->readBlock(16);
-    long long size = file->readQWORD();
+    if(guid.size() != 16) {
+      file->setValid(false);
+      break;
+    }
+    bool ok;
+    long long size = file->readQWORD(&ok);
+    if(!ok) {
+      file->setValid(false);
+      break;
+    }
     BaseObject *obj;
     if(guid == metadataGuid) {
       obj = new MetadataObject();
@@ -389,19 +406,37 @@ void ASF::File::read(bool /*readProperties*/, Properties::ReadStyle /*properties
   ByteVector guid = readBlock(16);
   if(guid != headerGuid) {
     debug("ASF: Not an ASF file.");
+    setValid(false);
     return;
   }
 
   d->tag = new ASF::Tag();
   d->properties = new ASF::Properties();
 
-  d->size = readQWORD();
-  int numObjects = readDWORD();
+  bool ok;
+  d->size = readQWORD(&ok);
+  if(!ok) {
+    setValid(false);
+    return;
+  }
+  int numObjects = readDWORD(&ok);
+  if(!ok) {
+    setValid(false);
+    return;
+  }
   seek(2, Current);
 
   for(int i = 0; i < numObjects; i++) {
     ByteVector guid = readBlock(16);
-    long size = (long)readQWORD();
+    if(guid.size() != 16) {
+      setValid(false);
+      break;
+    }
+    long size = (long)readQWORD(&ok);
+    if(!ok) {
+      setValid(false);
+      break;
+    }
     BaseObject *obj;
     if(guid == filePropertiesGuid) {
       obj = new FilePropertiesObject();
@@ -429,7 +464,12 @@ void ASF::File::read(bool /*readProperties*/, Properties::ReadStyle /*properties
 bool ASF::File::save()
 {
   if(readOnly()) {
-    debug("ASF: File is read-only.");
+    debug("ASF::File::save() -- File is read only.");
+    return false;
+  }
+
+  if(!isValid()) {
+    debug("ASF::File::save() -- Trying to save invalid file.");
     return false;
   }
 
@@ -491,27 +531,47 @@ bool ASF::File::save()
 // protected members
 ////////////////////////////////////////////////////////////////////////////////
 
-int ASF::File::readBYTE()
+int ASF::File::readBYTE(bool *ok)
 {
   ByteVector v = readBlock(1);
+  if(v.size() != 1) {
+    if(ok) *ok = false;
+    return 0;
+  }
+  if(ok) *ok = true;
   return v[0];
 }
 
-int ASF::File::readWORD()
+int ASF::File::readWORD(bool *ok)
 {
   ByteVector v = readBlock(2);
+  if(v.size() != 2) {
+    if(ok) *ok = false;
+    return 0;
+  }
+  if(ok) *ok = true;
   return v.toUShort(false);
 }
 
-unsigned int ASF::File::readDWORD()
+unsigned int ASF::File::readDWORD(bool *ok)
 {
   ByteVector v = readBlock(4);
+  if(v.size() != 4) {
+    if(ok) *ok = false;
+    return 0;
+  }
+  if(ok) *ok = true;
   return v.toUInt(false);
 }
 
-long long ASF::File::readQWORD()
+long long ASF::File::readQWORD(bool *ok)
 {
   ByteVector v = readBlock(8);
+  if(v.size() != 8) {
+    if(ok) *ok = false;
+    return 0;
+  }
+  if(ok) *ok = true;
   return v.toLongLong(false);
 }
 

taglib/asf/asffile.h

@@ -88,10 +88,10 @@ namespace TagLib {
 
     private:
 
-      int readBYTE();
-      int readWORD();
-      unsigned int readDWORD();
-      long long readQWORD();
+      int readBYTE(bool *ok = 0);
+      int readWORD(bool *ok = 0);
+      unsigned int readDWORD(bool *ok = 0);
+      long long readQWORD(bool *ok = 0);
       static ByteVector renderString(const String &str, bool includeLength = false);
       String readString(int len);
       void read(bool readProperties, Properties::ReadStyle propertiesStyle);

taglib/flac/flacfile.cpp

@@ -149,7 +149,7 @@ bool FLAC::File::save()
 
   // Create new vorbis comments
 
-  Tag::duplicate(&d->tag, xiphComment(true), true);
+  Tag::duplicate(&d->tag, xiphComment(true), false);
 
   d->xiphCommentData = xiphComment()->render(false);
 
@@ -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/mp4/mp4properties.cpp

@@ -91,15 +91,24 @@ MP4::Properties::Properties(File *file, MP4::Atoms *atoms, ReadStyle style)
 
   file->seek(mdhd->offset);
   data = file->readBlock(mdhd->length);
-  if(data[8] == 0) {
-    unsigned int unit = data.mid(20, 4).toUInt();
-    unsigned int length = data.mid(24, 4).toUInt();
-    d->length = length / unit;
-  }
-  else {
+  uint version = data[8];
+  if(version == 1) {
+    if (data.size() < 36 + 8) {
+      debug("MP4: Atom 'trak.mdia.mdhd' is smaller than expected");
+      return;
+    }
     long long unit = data.mid(28, 8).toLongLong();
     long long length = data.mid(36, 8).toLongLong();
-    d->length = int(length / unit);
+    d->length = unit ? int(length / unit) : 0;
+  }
+  else {
+    if (data.size() < 24 + 4) {
+      debug("MP4: Atom 'trak.mdia.mdhd' is smaller than expected");
+      return;
+    }
+    unsigned int unit = data.mid(20, 4).toUInt();
+    unsigned int length = data.mid(24, 4).toUInt();
+    d->length = unit ? length / unit : 0;
   }
 
   MP4::Atom *atom = trak->find("mdia", "minf", "stbl", "stsd");

taglib/mpeg/id3v2/id3v2header.cpp

@@ -23,7 +23,7 @@
  *   http://www.mozilla.org/MPL/                                           *
  ***************************************************************************/
 
-#include <ostream>
+#include <iostream>
 #include <bitset>
 
 #include <tstring.h>

taglib/mpeg/id3v2/id3v2synchdata.cpp

@@ -23,7 +23,7 @@
  *   http://www.mozilla.org/MPL/                                           *
  ***************************************************************************/
 
-#include <ostream>
+#include <iostream>
 
 #include "id3v2synchdata.h"
 

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

@@ -23,7 +23,7 @@
  *   http://www.mozilla.org/MPL/                                           *
  ***************************************************************************/
 
-#include <ostream>
+#include <iostream>
 
 #include <tstring.h>
 #include <tdebug.h>
@@ -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/toolkit/tbytevector.h

@@ -30,7 +30,7 @@
 #include "taglib_export.h"
 
 #include <vector>
-#include <ostream>
+#include <iostream>
 
 namespace TagLib {
 

taglib/toolkit/tstring.cpp

@@ -27,7 +27,7 @@
 #include "unicode.h"
 #include "tdebug.h"
 
-#include <ostream>
+#include <iostream>
 
 #include <string.h>
 

taglib/toolkit/tstring.h

@@ -31,7 +31,7 @@
 #include "tbytevector.h"
 
 #include <string>
-#include <ostream>
+#include <iostream>
 
 /*!
  * \relates TagLib::String

taglib/toolkit/tstringlist.h

@@ -31,7 +31,7 @@
 #include "tbytevectorlist.h"
 #include "taglib_export.h"
 
-#include <ostream>
+#include <iostream>
 
 namespace TagLib {
 

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_flac.cpp

@@ -5,6 +5,7 @@
 #include <tstringlist.h>
 #include <tbytevectorlist.h>
 #include <flacfile.h>
+#include <xiphcomment.h>
 #include "utils.h"
 
 using namespace std;
@@ -20,6 +21,7 @@ class TestFLAC : public CppUnit::TestFixture
   CPPUNIT_TEST(testReplacePicture);
   CPPUNIT_TEST(testRemoveAllPictures);
   CPPUNIT_TEST(testRepeatedSave);
+  CPPUNIT_TEST(testSaveMultipleValues);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -186,6 +188,26 @@ public:
     CPPUNIT_ASSERT_EQUAL(String("NEW TITLE 2"), tag->title());
   }
 
+  void testSaveMultipleValues()
+  {
+    ScopedFileCopy copy("silence-44-s", ".flac", false);
+    string newname = copy.fileName();
+
+    FLAC::File *f = new FLAC::File(newname.c_str());
+    Ogg::XiphComment* c = f->xiphComment(true);
+    c->addField("ARTIST", "artist 1", true);
+    c->addField("ARTIST", "artist 2", false);
+    f->save();
+    delete f;
+
+    f = new FLAC::File(newname.c_str());
+    c = f->xiphComment(true);
+    Ogg::FieldListMap m = c->fieldListMap();
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(2), m["ARTIST"].size());
+    CPPUNIT_ASSERT_EQUAL(String("artist 1"), m["ARTIST"][0]);
+    CPPUNIT_ASSERT_EQUAL(String("artist 2"), m["ARTIST"][1]);
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestFLAC);

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);