mirror/taglib
1
0
Fork 0
mirror of https://github.com/taglib/taglib.git synced 2026-11-06 22:48:48 -05:00
Code Issues Packages Projects Releases Wiki Activity

RIFF: Don't let a clamped chunk swallow chunks saved after it (#1482)

Browse Source 
Only the audio chunk (data, or SSND in AIFF) is clamped to the end of
the file when it declares more than the file holds, and its size is
rewritten before a chunk is appended after it. Any other chunk running
past the end, such as a trailing APEv2 tag, ends the chunk walk again,
so a save inserts new chunks ahead of it instead of losing them on the
next read.
This commit is contained in:
Ryan Francesconi
2026-11-01 06:39:07 +01:00
committed by GitHub
parent 80e42828a8
commit 96c7893332
2 changed files with 88 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
taglib/riff/rifffile.cpp
+24 -7
View File
@@ -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<unsigned int>(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<long long>(d->dataSize64);
}
bool clamped = false;
if(chunkSize > static_cast<long long>(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<offset_t>(chunkSize);
chunk.offset = offset + 8;
chunk.padding = 0;
chunk.clamped = clamped;
offset = chunk.offset + chunk.size;
tests/test_wav.cpp
+64 -1
View File
@@ -29,6 +29,7 @@
#include <cstdio>
#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<offset_t>(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");