Fix ID3v2 frame data length check for per-frame unsynchronised ID3v2.4 frames (#1385)

Frame::fieldData() (taglib/mpeg/id3v2/id3v2frame.cpp) discarded any
ID3v2.4 frame whose declared size (from the header) no longer matched
its actual buffer size after per-frame unsynchronisation was decoded by
FrameFactory::prepareFrameHeader(), silently emptying frames like
TIT2/TPE1/TALB. Clamp the declared length to what's actually available
instead of discarding the frame, only bailing out if the frame's data
offset itself doesn't fit.

Adds testUnsynchDecodeID3v24Frame() to tests/test_id3v2.cpp, covering a
frame with its own per-frame Unsynchronisation flag (as opposed to the
tag-wide flag already covered by testUnsynchDecode()), using new fixture
tests/data/unsynch24.id3.
This commit is contained in:
Thomas Bergwinkl
2026-08-01 07:53:53 +02:00
committed by GitHub
parent a100d0b2ec
commit 5530420d08
3 changed files with 15 additions and 2 deletions
+7 -2
View File
@@ -304,8 +304,13 @@ ByteVector Frame::fieldData(const ByteVector &frameData) const
frameDataOffset + frameDataLength > frameData.size()) {
// The first check is needed because some "dual purpose" frame constructors
// call this method with only the frame ID, i.e. without a complete header.
debug("Invalid frame data length");
return ByteVector();
if(frameDataOffset > frameData.size()) {
debug("Invalid frame data length");
return ByteVector();
}
// Per-frame ID3v2.4 unsynchronisation can shrink frameData after the
// header's declared size was set; use what's actually available.
frameDataLength = frameData.size() - frameDataOffset;
}
if(zlib::isAvailable() && d->header->compression() && !d->header->encryption()) {
Binary file not shown.
+8
View File
@@ -72,6 +72,7 @@ class TestID3v2 : public CppUnit::TestFixture
{
CPPUNIT_TEST_SUITE(TestID3v2);
CPPUNIT_TEST(testUnsynchDecode);
CPPUNIT_TEST(testUnsynchDecodeID3v24Frame);
CPPUNIT_TEST(testDowngradeUTF8ForID3v23_1);
CPPUNIT_TEST(testDowngradeUTF8ForID3v23_2);
CPPUNIT_TEST(testUTF16BEDelimiter);
@@ -151,6 +152,13 @@ public:
CPPUNIT_ASSERT_EQUAL(String("My babe just cares for me"), f.tag()->title());
}
void testUnsynchDecodeID3v24Frame()
{
MPEG::File f(TEST_FILE_PATH_C("unsynch24.id3"), false);
CPPUNIT_ASSERT(f.tag());
CPPUNIT_ASSERT_EQUAL(String("Hi"), f.tag()->title());
}
void testDowngradeUTF8ForID3v23_1()
{
ScopedFileCopy copy("xing", ".mp3");