From f59c3b67aa4eb4a6a651a70d457211eddbf53b12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= <lalinsky@gmail.com>
Date: Sat, 8 Oct 2011 18:41:15 +0200
Subject: [PATCH] Detect RIFF files with invalid chunk sizes

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
---
 taglib/riff/aiff/aifffile.cpp  |   5 +++++
 taglib/riff/rifffile.cpp       |  22 +++++++++++++++++++++-
 taglib/riff/wav/wavfile.cpp    |   5 +++++
 tests/data/zero-size-chunk.wav | Bin 0 -> 1024 bytes
 tests/test_wav.cpp             |   8 ++++++++
 5 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 tests/data/zero-size-chunk.wav

diff --git a/taglib/riff/aiff/aifffile.cpp b/taglib/riff/aiff/aifffile.cpp
index 425bfa02..72667f6e 100644
--- a/taglib/riff/aiff/aifffile.cpp
+++ b/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;
diff --git a/taglib/riff/rifffile.cpp b/taglib/riff/rifffile.cpp
index 8d23bcd6..a3ca0e3e 100644
--- a/taglib/riff/rifffile.cpp
+++ b/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;
     }
 
diff --git a/taglib/riff/wav/wavfile.cpp b/taglib/riff/wav/wavfile.cpp
index 9ec3b510..37d8a4d2 100644
--- a/taglib/riff/wav/wavfile.cpp
+++ b/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;
diff --git a/tests/data/zero-size-chunk.wav b/tests/data/zero-size-chunk.wav
new file mode 100644
index 0000000000000000000000000000000000000000..8517e797dc803bfbcea502588f70d68e643a6aad
GIT binary patch
literal 1024
zcmWIYbaUfiU|<M$40BD(Em05vvKbke7+lscFbHg9VqjqqU`R<UNdyXyg3%Bd4S``6
F0sw5+2R;A*

literal 0
HcmV?d00001

diff --git a/tests/test_wav.cpp b/tests/test_wav.cpp
index 38a9a0fb..0bd82018 100644
--- a/tests/test_wav.cpp
+++ b/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);