From 9dcdecc81058fb7e149d1b5b6ceb6172d5c27866 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Thu, 15 Apr 2010 20:22:21 +0000 Subject: [PATCH] Fix parsing of regular 32-bit integers in SynchData::toUInt() BUG:231075 git-svn-id: svn://anonsvn.kde.org/home/kde/trunk/kdesupport/taglib@1115275 283d02a7-25f6-0310-bc7c-ecb5cbfe19da --- NEWS | 4 +++- taglib/mpeg/id3v2/id3v2synchdata.cpp | 12 ++++-------- tests/data/compressed_id3_frame.mp3 | Bin 0 -> 5000 bytes tests/test_id3v2.cpp | 14 ++++++++++++++ tests/test_synchdata.cpp | 19 +++++++++++++++++++ 5 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 tests/data/compressed_id3_frame.mp3 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 0000000000000000000000000000000000000000..60bc895633b1ce32894e54800f6aa65d4e2cb546 GIT binary patch literal 5000 zcmeG=c{r5a+a{zE6A@(@MM(CwEMo~nCA}iqW*8|2sG#x6pQ zk;a(pJa%T7v3yUxe|+!tyS~4_zkbhko##I1xzD-JIrn})=EiF5Y~UlWZg|!F0vp>A zC$>m7$X{6@*$X)=hoiP9+7}|qBARzU==;`E+7~OTNTdy9t^HCLd%-b5P6xS{5@fkJ zGN)5ahl!!zDTc(;=Z?mTf2H6eqR=2RWkJ=Hz8Gb1fy z?flZ@<=)H37ATG0)$ORCbeLnMM}MjfZGO~2y@;{GR6}}(^)5(Xr-*y%t=R$S4L$HU?ezP)p9gIQTwd8g#K;QQ}hmHBE<h-Q zhV>B(G7gmr2Ko*5r+p)iC@H)qBgt-|L)RPnQW{5gErhZwbdsA8fk`BrKsR zobx2781mlrfuB)5V};%=rh&&vi)wPS5&6eWHN7W)0ViXy-t@PJFkiV{c77qe)o)AS z8Xm{%(5Dn`l=;mZH@fz+=WttQa3+oHUgWsE`Pccit+z*liWpI9Y$4ZQRE@azn!28x z7dHo}es_J#Zl~*yFcpC~tjmJm9xdKhF$!{Zg)f1TK4LfRiaXfGH>KHA-0>-2j-V$= zwC@sybBgSf=^?qeopM|;ktdJnyAi4aK96(xuOXL5)yNgM{1iQN!fv$f>uyd*d7Uk= z;l)*Q_h#G4jk+8mDYRa^S)b<2o1AoW$m?v^4{hC*E|W0$+6XCIK6XsjQ}0mIi%d(= zeEDb764RYo4{3TubpaNN@hnC|j5GlU3;W2a==jp5fGK8K)l|KaySh?R%`L#(>S)=t zEio6xhr^oyTY`TE*X*KY=yh@bR;vg9_aKuU*Ic6f&)O%e!;LDl8}maY$a?0+5DL{0 zc3wkc<5w$povWBbsTiRgT?{94K1m%`%QoQdNlQx`A0L;Llgs3P{hg8Nn zA|oyRRpcx5;~d8WOrMWfI!`g%_WJB#Lqh`>RD34r6NyAhPd`>yU0oe~`}XVxb9WaX zbg!&TMZgqc7)*P?C7Y(l4t=mc8Y0d6Nuubx*I79^^%E8@F0>>Ot!1Q@mDTm@*PWc4 zP^e^lB-%1p*AQU{HOO~?H+|1v<+_*w91l4cSPB3@VL`!JMaABZ4l^^ek0C2)H#aw9 zV~){RFJGcvUBSawR8;gszaGrY%BrocrAd`4CG0!YR$@SZ&Uv%cNdlOedHeqTKKmyZ zZ%_4PL`JfqP}G)KC=Q2XZEiYKs8s!P=Qe@iivIroGiS~$bIm|}eSHrcX=!Qc;1@_l zWdJVa60tM9DH$0VLW+(D4j*1$U0q#XCO3q$(iI#u{YMiSz9S!MYHAjSE2hUjN8|DM z{3|(8wq}-=2uBYO4}${+s&Vkw*4u~Nk}vZ#r8H=0Y9131V4GeUZ)Yr0YCQ+v+@2>D z*;mb>mMQ}#yEME>g{Vex*usyh?!m#MdJ^*ThhIQ+PU#ygLGXF@jr)e9kE&vCC?tye z=}K+Jj*CD!BI;ze{ZUzaQ><@Xd*_+7-ueWsV!QIN;d0Cj*9?bQM7R7d&c3vqxRdo6k zwzXR2J>*CEbeq}`ZfI!ORC2vQg~I?qxErmS0eOlBQLoybEij>FzrHCvt$+cPm1HG2 z3sB?WjA0u?@Cw6V$=i*2d7R^%M810&4d`KNr)(Ll?vA0vjg{GevDghs2=S`~sG_|& zI#3v#_z~su1{C1(Rv$sXZzBmCsmBdzf=(xslfOaVo!XCA4o?(*v|f+j`SE>POP`-Z z2}kePFhwAgO3qCQC`}r|qZ#O07NFoz-!l5`^dwV;L?i7w+lf;yBiGyxe zIphOJ_zdr0aBwj6z=5Wel9w;{jq80350bYfP`Ax80KQZeslZgdV6rR!2(vqY|(yfG+wcM?>`{0}76eJQ!qtUjuwk#|xh{*lu zMygB^oF>I8ZOx746^o+ubs@2VdIH^}9#Q=^VSRT75yi0~sH<1m2zwKqX~zL--O()Q z<_{^mb@tO0TA59cb1wg#8t&{YvAX^8PPz3 z{GSc8GL(ST*@fX^ZiX}deDq@*O-t(ktwbL;}r z^wdG}xI2%ndMmZn$!TpwBl7iknSe%kgjU0*x$V+;d-AYpq>`dyaY4aRH)!Q}pJ`AY z!W z;lrsXX8CX~=X=Bn$6$nAKG`J}M`>SQU*nxM>GJS;WS`NlQSQAspSv=2Uhp1%oZqA^ zsO+JW2}hj?Sy>6e^uw`Wj)hiY2Pp(`vG$Md9?DK5W93GAj^7r!c`h3OE zkvRG$+eKehwb_roUi?_Z4s)ZfPIn?)JIC+Vt*hFBtoVA1^DW{HdK3j8?GZ+8N^UOi zU`b)2@q49rEbTZD*^{eBe<7g^(P{J7eFQ3bMn=4KV^L8Ox%)!z#Fn#Ti1cwCr2QTB zGiMI^5x%UPCMZGAhxxjr(ZBj%A?5~c$qlx}j-4GHY&h(m=47a9itEtY@ETkk9`-Cy zHFqH^C55fFF`T8j8g^p5^TNZjb_6KyTTAa)y=CEGn0_?xk{HWtIE0-z*^X}nITs!t z9=tSaK^AuVfJ0`KtUcJHz0Xq{8qze4TohDXe)1eUb4^f5s}4Gj(Rg=$f> z|FuBO-s)uc%F4>xD?%lh|$;qjNP|yKo4sM6h>#$+p1_oaCd!Q)| z?mBIN&OPU3F;FSKs!3@CN0Ub$=lluO&omN6w!%WKx>{RDecC^|>Z1{!@W+opydH?h zs)-M5{`m1DJ3CvLKwaFZ!`^1P6aw$U4jdX#36(4pdUOs)HxsF*BVPD&Xr9&w<}2mp<<@dGGo9BS5kj(t^Epi_A{-4um8#78 zi~--SR4^7o--bbbo2*jh2OM@e7e7LM~Hz<{NjvBU>l%Jm;8yj0&%L0RM z@E*>c{EK?G-7426Q8Y!*-6X1}KJPb2%aaombps|kz>G!9f3yz2!{qU9Tz4w>-oG)c z48uR6_$ln<%bkd?kCPv6p-C3|k9_cDlv_|0M)|gO;oi%$$i3DZ5gs09U0XT-5Y*rM z-d}O4HJA7lz?(fn*c9AL`c6Rpt(S}{|^?AlkM{&4els)!!!_*?^ zy1!WVou-#R)dO7N+F?3~7So{Adc2G*T1gs5rgnYor%=9_2C44zm!kj5d=h?T2BXRC z9}r;0l>Bu36dld)P6%B$a&?u&g{C|5<1X>xRN_mMM@~38Iq~@SDgNx@t$RdH=$2-k zj`g3IaJbSaMZcJ)KsP_xuN+#BRQl5X`o|e?D?@t!h~?Ywb8p^Ahe=hP9}46z6H1d{ z74#p(U@%}_3ucf3H8FoqC z@`YN3(ig>VURA*8Vb5JiSGFGqj;liX zSXx5m#m6wAkcj)giN8`8X3fonT}3=|kML6rj{VI#bOf%AtY7HU_MG=yzla+nu;_W* zj(n)A*S%R&<@g<+S8U6s=)Q`7s@?x6O9JKoUM3c?ASKzq=S$ z4?lkeV;4VH%mqt}-zzx5hVgS}v$iw&P3q4A!^=is_b}LWZT#FYH$D9@ZgA_XCMw{U z-}5c}(RZxPtyRG;3~WXLE?A5Q#w8#~!rBAli;w2~$$|d? D0Kmmp literal 0 HcmV?d00001 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);