From 5530420d088fa25f72d58349c6cc118b8f22d9bf Mon Sep 17 00:00:00 2001
From: Thomas Bergwinkl <bergos@users.noreply.github.com>
Date: Sat, 1 Aug 2026 07:53:53 +0200
Subject: [PATCH] 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.
---
 taglib/mpeg/id3v2/id3v2frame.cpp |   9 +++++++--
 tests/data/unsynch24.id3         | Bin 0 -> 28 bytes
 tests/test_id3v2.cpp             |   8 ++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/unsynch24.id3

diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp
index dc3b6c3c..90befd30 100644
--- a/taglib/mpeg/id3v2/id3v2frame.cpp
+++ b/taglib/mpeg/id3v2/id3v2frame.cpp
@@ -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()) {
diff --git a/tests/data/unsynch24.id3 b/tests/data/unsynch24.id3
new file mode 100644
index 0000000000000000000000000000000000000000..7969dd7ed5d6ecc7d78cc669eaee25f66f9ea3fa
GIT binary patch
literal 28
fcmeZtF=k-^0ih7j5F;SX!NA1$pW&YeLnZ?NH*N&8

literal 0
HcmV?d00001

diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 9087b54e..fc90f4aa 100644
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -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");