diff --git a/taglib/riff/rifffile.cpp b/taglib/riff/rifffile.cpp index 46ae35d9..fcb8a88c 100644 --- a/taglib/riff/rifffile.cpp +++ b/taglib/riff/rifffile.cpp @@ -46,6 +46,8 @@ struct Chunk //! May exceed 32 bits for the "data" chunk of an RF64/BW64 file. offset_t size; unsigned int padding; + //! The audio chunk, declared larger than the file and read to its end instead. + bool clamped { false }; }; class RIFF::File::FilePrivate @@ -243,10 +245,18 @@ void RIFF::File::appendChunk(const ByteVector &name, const ByteVector &data) return; } - // Adjust the padding of the last chunk to place the new chunk at even position. - Chunk &last = d->chunks.back(); + // A clamped chunk gets its real size before anything follows it, or the next read + // takes the new chunk for part of it. + if(last.clamped && !d->isLongForm) { + insert(ByteVector::fromUInt(static_cast(last.size), d->endianness == BigEndian), + last.offset - 4, 4); + last.clamped = false; + } + + // Adjust the padding of the last chunk to place the new chunk at even position. + offset_t offset = last.offset + last.size + last.padding; if(offset & 1) { if(last.padding == 1) { @@ -386,13 +396,19 @@ void RIFF::File::read() : static_cast(d->dataSize64); } + bool clamped = false; if(chunkSize > static_cast(available)) { - // Clamp to available bytes rather than rejecting the chunk outright. - // Some encoders write a correct data chunk but with a slightly too-large - // declared size, or place the data chunk outside the declared RIFF boundary. - // Lenient parsers (ffmpeg, QuickTime) handle this by clamping; we do the same. + // Some encoders write a correct audio chunk with a slightly too-large declared + // size, or place it outside the declared RIFF boundary. Lenient parsers (ffmpeg, + // QuickTime) clamp it to the available bytes; we do the same. Any other chunk + // running past the end is trailing data, such as an APE tag, and not a chunk. + if(chnkName != (bigEndian ? "SSND" : "data")) { + debug("RIFF::File::read() -- Chunk '" + chnkName + "' has invalid size (larger than the file size)"); + break; + } debug("RIFF::File::read() -- Chunk '" + chnkName + "' is truncated; clamping size to available bytes."); - chunkSize = length() - offset - 8; + chunkSize = available; + clamped = true; } Chunk chunk; @@ -400,6 +416,7 @@ void RIFF::File::read() chunk.size = static_cast(chunkSize); chunk.offset = offset + 8; chunk.padding = 0; + chunk.clamped = clamped; offset = chunk.offset + chunk.size; diff --git a/tests/test_wav.cpp b/tests/test_wav.cpp index 08365129..a587ccd4 100644 --- a/tests/test_wav.cpp +++ b/tests/test_wav.cpp @@ -29,6 +29,7 @@ #include #include "taglib_config.h" +#include "apetag.h" #include "tcharset.h" #include "id3v2tag.h" #include "infotag.h" @@ -356,6 +357,8 @@ class TestWAV : public CppUnit::TestFixture CPPUNIT_TEST(testFuzzedFile1); CPPUNIT_TEST(testFuzzedFile2); CPPUNIT_TEST(testFileWithGarbageAppended); + CPPUNIT_TEST(testTrailingAPETagIsNotAChunk); + CPPUNIT_TEST(testSaveAfterOversizedDataChunk); CPPUNIT_TEST(testStripAndProperties); CPPUNIT_TEST(testPCMWithFactChunk); CPPUNIT_TEST(testWaveFormatExtensible); @@ -1243,7 +1246,7 @@ public: { FileStream stream(copy.fileName().c_str()); stream.seek(0, IOStream::End); - constexpr char garbage[] = "\r2345678"; + constexpr char garbage[] = "12345678"; stream.writeBlock(ByteVector(garbage, sizeof(garbage) - 1)); stream.seek(0); contentsBeforeModification = stream.readBlock(stream.length()); @@ -1266,6 +1269,66 @@ public: } } + void testTrailingAPETagIsNotAChunk() + { + ScopedFileCopy copy("empty", ".wav"); + APE::Tag ape; + ape.setTitle("APE Title"); + const ByteVector apeData = ape.render(); + { + FileStream stream(copy.fileName().c_str()); + stream.seek(0, IOStream::End); + stream.writeBlock(apeData); + } + { + RIFF::WAV::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.isValid()); + f.ID3v2Tag()->setTitle("ID3v2 Title"); + f.InfoTag()->setTitle("INFO Title"); + CPPUNIT_ASSERT(f.save()); + } + { + RIFF::WAV::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT_EQUAL(String("ID3v2 Title"), f.ID3v2Tag()->title()); + CPPUNIT_ASSERT_EQUAL(String("INFO Title"), f.InfoTag()->title()); + } + { + FileStream stream(copy.fileName().c_str()); + stream.seek(-static_cast(apeData.size()), IOStream::End); + CPPUNIT_ASSERT_EQUAL(apeData, stream.readBlock(apeData.size())); + } + } + + void testSaveAfterOversizedDataChunk() + { + ScopedFileCopy copy("empty", ".wav"); + { + // empty.wav ends with its "data" chunk; declare 100 bytes more than it holds. + FileStream stream(copy.fileName().c_str()); + stream.seek(40); + const unsigned int dataSize = stream.readBlock(4).toUInt(false); + stream.seek(40); + stream.writeBlock(ByteVector::fromUInt(dataSize + 100, false)); + } + { + RIFF::WAV::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.isValid()); + f.ID3v2Tag()->setTitle("ID3v2 Title"); + f.InfoTag()->setTitle("INFO Title"); + CPPUNIT_ASSERT(f.save()); + } + { + RIFF::WAV::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT_EQUAL(String("ID3v2 Title"), f.ID3v2Tag()->title()); + CPPUNIT_ASSERT_EQUAL(String("INFO Title"), f.InfoTag()->title()); + RIFF::WAV::File original(TEST_FILE_PATH_C("empty.wav")); + CPPUNIT_ASSERT_EQUAL(original.audioProperties()->lengthInMilliseconds(), + f.audioProperties()->lengthInMilliseconds()); + } + } + void testStripAndProperties() { ScopedFileCopy copy("empty", ".wav");