From 934ce51790046b2f18c2e0e57a1078f62c4f7920 Mon Sep 17 00:00:00 2001 From: Scott Wheeler Date: Mon, 9 May 2011 19:06:08 +0200 Subject: [PATCH 1/8] Don't lead the scanned blocks on save --- taglib/flac/flacfile.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index f882ae7b..4e3d2b36 100644 --- a/taglib/flac/flacfile.cpp +++ b/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; } From b7ec0d26ab8e474f0ff77875b75158cb4345ed07 Mon Sep 17 00:00:00 2001 From: Frank Lai Date: Thu, 9 Jun 2011 18:44:54 +0200 Subject: [PATCH 2/8] Be more careful when parsing Vorbis Comments --- taglib/ogg/xiphcomment.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 344d9cfc..e7e8fa91 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -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); From 294cb222414292a03a5dd69931d365f53a0356cf Mon Sep 17 00:00:00 2001 From: "Stephen F. Booth" Date: Thu, 28 Jul 2011 08:36:14 -0400 Subject: [PATCH 3/8] Don't crash when wav files have a 0 for bit per channel (sampleWidth) I've seen this in a wav that has an audio format of MP3 (0x55) --- taglib/riff/aiff/aiffproperties.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/riff/aiff/aiffproperties.cpp b/taglib/riff/aiff/aiffproperties.cpp index 77c3d277..afb741b7 100644 --- a/taglib/riff/aiff/aiffproperties.cpp +++ b/taglib/riff/aiff/aiffproperties.cpp @@ -149,5 +149,5 @@ void RIFF::AIFF::Properties::read(const ByteVector &data) double sampleRate = ConvertFromIeeeExtended(reinterpret_cast(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; } From f59c3b67aa4eb4a6a651a70d457211eddbf53b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Sat, 8 Oct 2011 18:41:15 +0200 Subject: [PATCH 4/8] 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|length()); } + void testZeroSizeDataChunk() + { + RIFF::WAV::File f("data/zero-size-chunk.wav"); + CPPUNIT_ASSERT_EQUAL(false, f.isValid()); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestWAV); From 23c86cf27d625d25318a1897029922302c9d21a8 Mon Sep 17 00:00:00 2001 From: "Stephen F. Booth" Date: Sat, 4 Feb 2012 08:39:45 -0500 Subject: [PATCH 5/8] Check if the header is TTA1 before parsing --- taglib/trueaudio/trueaudioproperties.cpp | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/taglib/trueaudio/trueaudioproperties.cpp b/taglib/trueaudio/trueaudioproperties.cpp index 5b1bf12d..3cab855b 100644 --- a/taglib/trueaudio/trueaudioproperties.cpp +++ b/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; + } } From df1d3e028e467014991cc154caa1f7a313bce969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Sun, 4 Mar 2012 11:51:05 +0100 Subject: [PATCH 6/8] Make sure to not try dividing by zero --- taglib/ape/apeproperties.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/ape/apeproperties.cpp b/taglib/ape/apeproperties.cpp index 3154d104..7f9cdb5a 100644 --- a/taglib/ape/apeproperties.cpp +++ b/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; } From 258ae751b5a40255699c170c57400b19006d1c8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Sun, 4 Mar 2012 12:01:21 +0100 Subject: [PATCH 7/8] Don't store the output of ByteVector::toUInt() in int, use uint instead --- taglib/ogg/xiphcomment.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index e7e8fa91..b9f408f6 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/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); From 110cac8429bdf660e7749ced7054b0d1728df75c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Sat, 10 Mar 2012 08:46:20 +0100 Subject: [PATCH 8/8] Avoid uint overflow in case the length + index is over UINT_MAX --- taglib/toolkit/tbytevector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 9fb77b12..e5dcf1d8 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/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();