diff --git a/NEWS b/NEWS index 3b5e6dbe..f629c77f 100644 --- a/NEWS +++ b/NEWS @@ -1,10 +1,12 @@ -TagLib 1.6.3 (Apr 13, 2010) +TagLib 1.6.3 (Apr 15, 2010) =========================== * Fixed definitions of the TAGLIB_WITH_MP4 and TAGLIB_WITH_ASF macros. * Fixed upgrading of ID3v2.3 genre frame with ID3v1 code 0 (Blues). * New method `int String::toInt(bool *ok)` which can return whether the conversion to a number was successfull. + * Fixed parsing of incorrectly written lengths in ID3v2 (affects mainly + compressed frames). (BUG:231075) TagLib 1.6.2 (Apr 9, 2010) ========================== diff --git a/taglib/mpeg/id3v2/id3v2synchdata.cpp b/taglib/mpeg/id3v2/id3v2synchdata.cpp index 6241751e..b150948f 100644 --- a/taglib/mpeg/id3v2/id3v2synchdata.cpp +++ b/taglib/mpeg/id3v2/id3v2synchdata.cpp @@ -46,14 +46,10 @@ TagLib::uint SynchData::toUInt(const ByteVector &data) } if(notSynchSafe) { - /* - * Invalid data; assume this was created by some buggy software that just - * put normal integers here rather than syncsafe ones, and try it that - * way. - */ - sum = 0; - for(int i = 0; i <= last; i++) - sum |= data[i] << ((last - i) * 8); + // Invalid data; assume this was created by some buggy software that just + // put normal integers here rather than syncsafe ones, and try it that + // way. + sum = (data.size() > 4) ? data.mid(0, 4).toUInt() : data.toUInt(); } return sum; diff --git a/tests/data/compressed_id3_frame.mp3 b/tests/data/compressed_id3_frame.mp3 new file mode 100644 index 00000000..60bc8956 Binary files /dev/null and b/tests/data/compressed_id3_frame.mp3 differ diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 860e9119..4276963d 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -61,6 +61,7 @@ class TestID3v2 : public CppUnit::TestFixture CPPUNIT_TEST(testUpdateGenre24); CPPUNIT_TEST(testUpdateDate22); // CPPUNIT_TEST(testUpdateFullDate22); TODO TYE+TDA should be upgraded to TDRC together + CPPUNIT_TEST(testCompressedFrameWithBrokenLength); CPPUNIT_TEST_SUITE_END(); public: @@ -455,6 +456,19 @@ public: CPPUNIT_ASSERT_EQUAL(String("2010-04-03"), f.ID3v2Tag()->frameListMap()["TDRC"].front()->toString()); } + void testCompressedFrameWithBrokenLength() + { + MPEG::File f("data/compressed_id3_frame.mp3", false); + CPPUNIT_ASSERT(f.ID3v2Tag()->frameListMap().contains("APIC")); + ID3v2::AttachedPictureFrame *frame = + static_cast(f.ID3v2Tag()->frameListMap()["APIC"].front()); + CPPUNIT_ASSERT(frame); + CPPUNIT_ASSERT_EQUAL(String("image/bmp"), frame->mimeType()); + CPPUNIT_ASSERT_EQUAL(ID3v2::AttachedPictureFrame::Other, frame->type()); + CPPUNIT_ASSERT_EQUAL(String(""), frame->description()); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(86414), frame->picture().size()); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v2); diff --git a/tests/test_synchdata.cpp b/tests/test_synchdata.cpp index 04ef5359..1113ee43 100644 --- a/tests/test_synchdata.cpp +++ b/tests/test_synchdata.cpp @@ -34,6 +34,8 @@ class TestID3v2SynchData : public CppUnit::TestFixture CPPUNIT_TEST(test1); CPPUNIT_TEST(test2); CPPUNIT_TEST(test3); + CPPUNIT_TEST(testToUIntBroken); + CPPUNIT_TEST(testToUIntBrokenAndTooLarge); CPPUNIT_TEST(testDecode1); CPPUNIT_TEST(testDecode2); CPPUNIT_TEST_SUITE_END(); @@ -67,6 +69,23 @@ public: CPPUNIT_ASSERT_EQUAL(ID3v2::SynchData::fromUInt(129), v); } + void testToUIntBroken() + { + char data[] = { 0, 0, 0, -1 }; + char data2[] = { 0, 0, -1, -1 }; + + CPPUNIT_ASSERT_EQUAL(TagLib::uint(255), ID3v2::SynchData::toUInt(ByteVector(data, 4))); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(65535), ID3v2::SynchData::toUInt(ByteVector(data2, 4))); + } + + void testToUIntBrokenAndTooLarge() + { + char data[] = { 0, 0, 0, -1, 0 }; + ByteVector v(data, 5); + + CPPUNIT_ASSERT_EQUAL(TagLib::uint(255), ID3v2::SynchData::toUInt(v)); + } + void testDecode1() { ByteVector a("\xff\x00\x00", 3);