From 900adcd5a852a81e4ad8b876fea696c843964304 Mon Sep 17 00:00:00 2001 From: Andreas Huggel Date: Sat, 22 Jul 2006 02:55:35 +0000 Subject: [PATCH] Added check for JPEG APP segments >64k, removed 16bit size limitation for IRBs. Fixes bug #480. --- src/Makefile | 1 + src/error.cpp | 1 + src/jpgimage.cpp | 50 ++++++++++++++----------- src/jpgimage.hpp | 8 ++-- src/largeiptc-test.cpp | 71 ++++++++++++++++++++++++++++++++++++ src/tiffvisitor.cpp | 4 +- test/bugfixes-test.sh | 4 ++ test/data/bugfixes-test.out | 8 ++++ test/data/exiv2-bug480.jpg | Bin 0 -> 4745 bytes 9 files changed, 119 insertions(+), 28 deletions(-) create mode 100644 src/largeiptc-test.cpp create mode 100644 test/data/exiv2-bug480.jpg diff --git a/src/Makefile b/src/Makefile index 1fc049ac..d533d7e6 100644 --- a/src/Makefile +++ b/src/Makefile @@ -110,6 +110,7 @@ BINSRC = addmoddel.cpp \ iptcprint.cpp \ iptctest.cpp \ key-test.cpp \ + largeiptc-test.cpp \ makernote-test.cpp \ taglist.cpp \ write-test.cpp \ diff --git a/src/error.cpp b/src/error.cpp index 57e7ae74..5f9fd075 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -78,6 +78,7 @@ namespace Exiv2 { ErrMsg( 34, "%1: Not supported"), // %1=function ErrMsg( 35, "ImageFactory registry full"), ErrMsg( 36, "Failed to decode %1 metadata"), // %1=type of metadata (Exif, IPTC) + ErrMsg( 37, "Size of %1 JPEG segment is larger than 65535 bytes"), // %1=type of metadata (Exif, IPTC, JPEG comment) // Last error message (message is not used) ErrMsg( -2, "(Unknown Error)") diff --git a/src/jpgimage.cpp b/src/jpgimage.cpp index 17e7ce66..2211c77c 100644 --- a/src/jpgimage.cpp +++ b/src/jpgimage.cpp @@ -71,8 +71,8 @@ namespace Exiv2 { long sizePsData, uint16_t psTag, const byte** record, - uint16_t *const sizeHdr, - uint16_t *const sizeData) + uint32_t *const sizeHdr, + uint32_t *const sizeData) { assert(record); assert(sizeHdr); @@ -95,12 +95,12 @@ namespace Exiv2 { if (position >= sizePsData) return -2; // Data is also padded to be even - long dataSize = getULong(pPsData + position, bigEndian); + uint32_t dataSize = getULong(pPsData + position, bigEndian); position += 4; - if (dataSize > sizePsData - position) return -2; + if (dataSize > static_cast(sizePsData - position)) return -2; if (type == psTag) { - *sizeData = static_cast(dataSize); + *sizeData = dataSize; *sizeHdr = psSize + 10; *record = hrd; return 0; @@ -113,8 +113,8 @@ namespace Exiv2 { int Photoshop::locateIptcIrb(const byte* pPsData, long sizePsData, const byte** record, - uint16_t *const sizeHdr, - uint16_t *const sizeData) + uint32_t *const sizeHdr, + uint32_t *const sizeData) { return locateIrb(pPsData, sizePsData, iptc_, record, sizeHdr, sizeData); @@ -131,17 +131,17 @@ namespace Exiv2 { else hexdump(std::cerr, pPsData, sizePsData); #endif const byte* record = pPsData; - uint16_t sizeIptc = 0; - uint16_t sizeHdr = 0; + uint32_t sizeIptc = 0; + uint32_t sizeHdr = 0; // Safe to call with zero psData.size_ Photoshop::locateIptcIrb(pPsData, sizePsData, &record, &sizeHdr, &sizeIptc); Blob psBlob; // Data is rounded to be even - const uint16_t sizeOldData = sizeHdr + sizeIptc + (sizeIptc & 1); - const uint16_t sizeFront = static_cast(record - pPsData); - const uint16_t sizeEnd = static_cast(sizePsData - sizeFront - sizeOldData); + const uint32_t sizeOldData = sizeHdr + sizeIptc + (sizeIptc & 1); + const uint32_t sizeFront = record - pPsData; + const uint32_t sizeEnd = sizePsData - sizeFront - sizeOldData; // Write data before old record. if (sizePsData > 0 && sizeFront > 0) { @@ -309,8 +309,8 @@ namespace Exiv2 { io_->read(psData.pData_, psData.size_); if (io_->error() || io_->eof()) throw Error(14); const byte *record = 0; - uint16_t sizeIptc = 0; - uint16_t sizeHdr = 0; + uint32_t sizeIptc = 0; + uint32_t sizeHdr = 0; // Find actual Iptc data within the APP13 segment if (!Photoshop::locateIptcIrb(psData.pData_, psData.size_, &record, &sizeHdr, &sizeIptc)) { @@ -465,8 +465,10 @@ namespace Exiv2 { // Write COM marker, size of comment, and string tmpBuf[0] = 0xff; tmpBuf[1] = com_; - us2Data(tmpBuf + 2, - static_cast(comment_.length()+3), bigEndian); + + if (comment_.length() + 3 > 0xffff) throw Error(37, "JPEG comment"); + us2Data(tmpBuf + 2, static_cast(comment_.length() + 3), bigEndian); + if (outIo.write(tmpBuf, 4) != 4) throw Error(21); if (outIo.write((byte*)comment_.data(), (long)comment_.length()) != (long)comment_.length()) throw Error(21); @@ -476,15 +478,17 @@ namespace Exiv2 { } if (exifData_.count() > 0) { // Write APP1 marker, size of APP1 field, Exif id and Exif data - DataBuf rawExif(exifData_.copy()); + DataBuf rawExif = exifData_.copy(); tmpBuf[0] = 0xff; tmpBuf[1] = app1_; - us2Data(tmpBuf + 2, - static_cast(rawExif.size_+8), - bigEndian); + + if (rawExif.size_ + 8 > 0xffff) throw Error(37, "Exif"); + us2Data(tmpBuf + 2, static_cast(rawExif.size_ + 8), bigEndian); memcpy(tmpBuf + 4, exifId_, 6); if (outIo.write(tmpBuf, 10) != 10) throw Error(21); - if (outIo.write(rawExif.pData_, rawExif.size_) + + // Write new Exif data buffer + if ( outIo.write(rawExif.pData_, rawExif.size_) != rawExif.size_) throw Error(21); if (outIo.error()) throw Error(21); --search; @@ -499,6 +503,8 @@ namespace Exiv2 { // Write APP13 marker, new size, and ps3Id tmpBuf[0] = 0xff; tmpBuf[1] = app13_; + + if (newPsData.size_ + 16 > 0xffff) throw Error(37, "IPTC"); us2Data(tmpBuf + 2, static_cast(newPsData.size_ + 16), bigEndian); memcpy(tmpBuf + 4, Photoshop::ps3Id_, 14); if (outIo.write(tmpBuf, 18) != 18) throw Error(21); @@ -506,7 +512,7 @@ namespace Exiv2 { // Write new Photoshop IRB data buffer if ( outIo.write(newPsData.pData_, newPsData.size_) - != newPsData.size_) throw Error(21); + != newPsData.size_) throw Error(21); if (outIo.error()) throw Error(21); } if (iptcData_.count() > 0) { diff --git a/src/jpgimage.hpp b/src/jpgimage.hpp index 2e6a30e7..ea244c95 100644 --- a/src/jpgimage.hpp +++ b/src/jpgimage.hpp @@ -87,16 +87,16 @@ namespace Exiv2 { long sizePsData, uint16_t psTag, const byte **record, - uint16_t *const sizeHdr, - uint16_t *const sizeData); + uint32_t *const sizeHdr, + uint32_t *const sizeData); /*! @brief Forwards to locateIrb() with \em psTag = \em iptc_ */ static int locateIptcIrb(const byte *pPsData, long sizePsData, const byte **record, - uint16_t *const sizeHdr, - uint16_t *const sizeData); + uint32_t *const sizeHdr, + uint32_t *const sizeData); /*! @brief Set the new IPTC IRB, keeps existing IRBs but removes the IPTC block if there is no new IPTC data to write. diff --git a/src/largeiptc-test.cpp b/src/largeiptc-test.cpp new file mode 100644 index 00000000..a9075e62 --- /dev/null +++ b/src/largeiptc-test.cpp @@ -0,0 +1,71 @@ +// ***************************************************************** -*- C++ -*- +// Test for large (>65535 bytes) IPTC buffer + +#include "iptc.hpp" +#include "image.hpp" +#include "jpgimage.hpp" +#include "futils.hpp" +#include +#include + +int main(int argc, char* const argv[]) +try { + if (argc != 3) { + std::cout << "Usage: " << argv[0] << " image datafile\n"; + return 1; + } + std::string file(argv[1]); + std::string data(argv[2]); + + // Read data file into data buffer + Exiv2::FileIo io(data); + if (io.open() != 0) { + throw Exiv2::Error(9, io.path(), Exiv2::strError()); + } + Exiv2::DataBuf buf(io.size()); + std::cout << "Reading " << buf.size_ << " bytes from " << data << "\n"; + io.read(buf.pData_, buf.size_); + if (io.error() || io.eof()) throw Exiv2::Error(14); + + // Read metadata from file + Exiv2::Image::AutoPtr image = Exiv2::ImageFactory::open(file); + assert(image.get() != 0); + image->readMetadata(); + + // Set Preview field to the content of the data file + Exiv2::DataValue value; + value.read(buf.pData_, buf.size_); + Exiv2::IptcData& iptcData = image->iptcData(); + std::cout << "IPTC fields: " << iptcData.size() << "\n"; + iptcData["Iptc.Application2.Preview"] = value; + std::cout << "IPTC fields: " << iptcData.size() << "\n"; + + // Set IRB, compare with IPTC raw data + Exiv2::DataBuf irb = Exiv2::Photoshop::setIptcIrb(0, 0, iptcData); + std::cout << "IRB buffer : " << irb.size_ << "\n"; + const Exiv2::byte* record; + uint32_t sizeHdr; + uint32_t sizeData; + Exiv2::Photoshop::locateIptcIrb(irb.pData_, irb.size_, &record, &sizeHdr, &sizeData); + Exiv2::DataBuf rawIptc = iptcData.copy(); + std::cout << "Comparing IPTC and IRB size... "; + if (static_cast(rawIptc.size_) != sizeData) { + std::cout << "not "; + } + std::cout << "ok\n"; + + std::cout << "Comparing IPTC and IRB data... "; + if (0 != memcmp(rawIptc.pData_, record + sizeHdr, sizeData)) { + std::cout << "not "; + } + std::cout << "ok\n"; + + // Set Iptc data and write it to the file + image->writeMetadata(); + + return 0; +} +catch (Exiv2::AnyError& e) { + std::cout << "Caught Exiv2 exception '" << e << "'\n"; + return -1; +} diff --git a/src/tiffvisitor.cpp b/src/tiffvisitor.cpp index c830ce94..d189f080 100644 --- a/src/tiffvisitor.cpp +++ b/src/tiffvisitor.cpp @@ -190,8 +190,8 @@ namespace Exiv2 { assert(pImage_ != 0); if (!object->pData()) return; byte const* record = 0; - uint16_t sizeHdr = 0; - uint16_t sizeData = 0; + uint32_t sizeHdr = 0; + uint32_t sizeData = 0; if (0 != Photoshop::locateIptcIrb(object->pData(), object->size(), &record, &sizeHdr, &sizeData)) { return; diff --git a/test/bugfixes-test.sh b/test/bugfixes-test.sh index b52b7ebe..fe1a522c 100755 --- a/test/bugfixes-test.sh +++ b/test/bugfixes-test.sh @@ -58,6 +58,10 @@ num=479 filename=`prep_file $num` $binpath/exiv2 -pt $filename +num=480 +filename=`prep_file $num` +$binpath/largeiptc-test $filename ../data/smiley1.jpg.ixgd + ) > $results 2>&1 if [ x`which unix2dos.exe` != x ]; then diff --git a/test/data/bugfixes-test.out b/test/data/bugfixes-test.out index ca368b85..a8a1f0ca 100644 --- a/test/data/bugfixes-test.out +++ b/test/data/bugfixes-test.out @@ -235,3 +235,11 @@ Exif.Image.0x9286 Undefined 264 (Binary value suppre Exif.Photo.ExifVersion Undefined 4 48 50 50 48 Exif.Photo.PixelXDimension Short 1 3173 Exif.Photo.PixelYDimension Short 1 2011 +------> Bug 480 <------- +Reading 67070 bytes from ../data/smiley1.jpg.ixgd +IPTC fields: 0 +IPTC fields: 67079 +IRB buffer : 67092 +Comparing IPTC and IRB size... ok +Comparing IPTC and IRB data... ok +Caught Exiv2 exception 'Size of IPTC JPEG segment is larger than 65535 bytes' diff --git a/test/data/exiv2-bug480.jpg b/test/data/exiv2-bug480.jpg new file mode 100644 index 0000000000000000000000000000000000000000..9394df8dd3c7379b76c7d20a5abcbd88c46ec08c GIT binary patch literal 4745 zcmb7DcR196|Noq`viII+kHaC#DA}iT&L#?H6xmT$BF?-s@5q*UXGCR{m9rB?Ns-7X z7m>Yv`~IHi_xu0%djI!Y@8@|x&-;A(d>&wf8=DyeAP@*J{Wrk*3}67zQq$1VP}9=V z(9+S-(lf9zFkHC6z{SGK#Kz0T$H&XX!y_OpB`P2Y5#r$yQxt{D9Nfw z$;!$8w**8-N5??Vz{$YCDJ#GuAp8Huc@Mx&58MC&{bvLI@nC9pfQCbymQz(9FffQkx4^Uv{*`yYU)0Cq4n zhq$V~1Sb~^MdKE9{uN;Qx0H$<%nn=u#?fxy161Q|ki!W}2Aw^0R3MR!FnV%~r2xzF zYxIcCPhyFwrCr*%OnI$|d$wWy=dzY_;KsbsJrwU{8}&poA2AbX3adhkH%5hgF(V%#1s7DVLd`D=&Aph-T%Yf3kGb z2b=mPR^q&6gZGK_3y5=DC(Cu-o zJZv%t396`!$a7G>v`Dj4u-pBeLC2uL8d_n5ub?7ND+CnN1?~ptmvbW|@mQ5fq5Cg< z_7~mJ_XkHSf7bL5Y+7Gz|M>J1;3rTdmC$m9NrKGna-GP8GE=Po0G~U!KUO1gx1)}M z`r{*hswUT0>E| z-Q$Rx7s)w9;kW4>kp5nJPUTUk+~?bsvw0`!#}B7t;)t0em&6lMX^%0mMaR4WXLcrU z>(*S11`$+)y@uzkUV^xi4`c3zk8e2yotaxKJ9~_%8E{g2e6|?)Q*j#A_|VWXnE-nR znuVQq;M7K(2$Bja)OYo@T!%xfuZ><qLzM|<6UiY-E~xqsiMt*7yg0f>068Dc}jN;Hc9sbWHz>c zZSx;>h%}db>Zh=u7E!u4}flRtd;3n!L7;)JvROK}(M7&LMpYCi- zYUfFzxfv2+giwGC-_NKGs4o3J`h6J-j<=I~_rW_QM?sEih(On{cS}+2r;cdu&mj?; zYR53p+C^~bw&0nb#H~PLq%Ul zCPumIAad+H_;)?{I}fSkWUZv9dTzxVq_ z7RTSdu)3_ADJdawJI7Y4&tbxUBAFEbvILmq@FXgm&b9Q51ZtvY!Kq(zH16$e8Rg;$ z4X8V$%1q0Wy&4(MUOd#ynmGdLbf=KE-KsxU_z0YKkZJwvCG5?WHC>gcZVZ3wLtTqv zH{FfSt~|$GehMi6@}0Z1Yf11KmP;iN4bxKF50Ww6%xb zVV9-8(cIE~>x$4#Vfdn`OWy0Q(Zl%%^+|vEHWkN|{l@m!wMHQjX6ut}Iuu0LzyWjp2<_He_rtWFLjWwX$d>s%E zm0N(`f>kRpl=`~?0q20|4D(}+I>x6059ZU>D{B}<{y#iBev-bwL$k|SYIqjMBeC6Ggh4adYA|#~hiq9DIl=Vy^!l6d{QO~?%p6CZ zq()E?G)h{`^pg`^R z6wmQBNzAdszPeb2a*uQl-1b4;GPJH3MW}S#4P|fd(q{blNzEuU^IGF{X4@b--vDfd zxhC0xxUQy?U+1)Z)uZtjIl4u2%G+Xoz>^@GLp!BV79Qb}77eQ&$holEY;a9({#`nzYt8jggp+1omi8 zRn(G#XS+(|Yw^S~B_Xe`mX3(}n3&>&PP^LhW|eaw>4((vtQxXE@Ey{{j%Zn3K+JV) z4GD}m#wpqbnjDDOh~zs~#O>qVg-Suc5Dmz_^{DWLVJVD5WtG9GK~U?>K2ZoZxX88c zZQgY{s7=e`1z)zP!Ry`w?{a@ooUpT_*|J08q$0660_@$73ypseV|4d0kU1ub_bNJY{?2EvofuV7^RA3&H0*Aw%)=1FuimcyDLp&ekZ#}O zCbKFpzs@&4sPUkbbm*|X3Qo3aH_4@EPv0`&XB6)bDAy3?hC?`!KA9i;R&bRrSmO{a zIq#-%i*xx$xJUNiaO`@Ftn}3SB=bqw-h9Uj2{Dvptb8&?DTqc63N+mw@@W%nqeN~# zWAZ%2i;$f9Xb?*bWv^0k2kpg68B2JT9uKwTYnJZ3r`dyp3w1!5ltz=AG;+Gk(fEm^&e?obX*@VkZW#drp zE5z~jkBb&Oie}PkyhAJKCu3&kK*q{50{p2AF(V%B2}b?8dW3GAG%^`Z+5gtsonEZX zKNF;hvJFCi=ta`TaH&CzSTJ2V;I9%rnICrtu!B3FV#xk4eGHqK1!%E#Ha_i|`%IF7 zo z(D;n}sr{kn&38)9LT%ARZXs2@YtxAn%7T-Ys1SZf4u!1`O2K_>$}z18Cyi@-{4+#N zdzill@tw~WWp~843iI61J7vvOGL}g5N(GqVGhwy1>a8-X49gMBtd2ZfM9%A+K6oro zaH?Lmn|+xnG0o^Nj7*OV7gGAWVg|jD|L?WQd~CMuIM?zoW=FH(nk%A0p_z^#Qbpsjl~?7=zknCJrfpadUa7x;>8c0R!{96=W3DNT;LHp&Pv zwKar_5n9_Qa?GR`VKfYDUwyudBS_(eE58Jwt3J0{``+LkXl5MSyid+!l z)fH0FtqAd_iYq&s(tj43SQiILX2oM` z=KY;7ms76_Tb$_zepSf%YJ?=O?=YYr)FZE%0kP4BTzH6ce+L-R_laFX*m zV7cF0C?mfawacyka?2J@u~9Iq5M@s6rp`^>zGdsv`gYZ_qpmVEi!P{5)TGF zFDP8U(f{IVIRt-QlN~lhKVy&HT*YK&-=-s8@`tkeYmGuO65mQdU&Ed-cQ>e&z&}Ol z_iv-DO$;UCq<;qg$Xd$kyz4i~+`MJa;^VCj4cPU_t$I>}c+HY28- z!RcKjd&rWGj%sAQS{hlCm~{c=$i>64GdZubX<-4Wu5KWtyz;E5OI!6Uncf%Nfs7p} zPY>}u67$>RS$HTqtmWh3a3}l|gW`lXhrlTvetiMMw4sYL72(8RHHd`U>;{Mwh4lzT zR1o9Mxf;ieXNP804%3V`P|!+tBDn11#r*{nn;z7jaE7eGmjKRB%ZE`3^C@@uz!kc6 zev`)=32AS2>PNS8h@?jb7t(Wv>P!W*V^)($lB8r@r5JRHq?TT)1-31=nyodHZdgE{ zt|#!0{ODkF$b#skC518)n$z?8f3@XD6(=X?ULLJ%Zm5=+b1<&>A>a|IZ3Dx7r8@^4 z_D0(>h4{88!HHGvxTn!Qe*1mb9ic}V{34#W^n8oP^SyYmOS*htY{GyNMcYxWI84qr zQfA4a+urzOK!e)JoD-oT_jA#5j_Byp+_$d0dJ8N{J?L89L^}hC<_}UIyJ(2rSy=)Q znu(oaN-}jR5f^0afESl8_}-k;B2-TldyE-|b1S{}&1pJzHMS);OQ^@9tY%)QQX&_M za}(~M<94YWJv;Zl1Z9>Wld+8Ppez)rq>esrG{yZ$P%XKoyf~q;ErjWeZFEgclq?3usX9x&kU6 z&BVj=%-P*OBQ~}N0(w5wdW4MjZqWwwl>{COiPxbe!)0-%I7e=E{%CngQ*OWap?V(k ztd*~173`Hr3>@K>4c*KadGuWL9=MJG!J!+zP6u|2!xdSzgg!lI7vDKdFVC>Z1IxRG3CdQv+apUGPEJK#Z*b^g3R?)Z)c2*10J`*Rz{zt|@Ex zQG0y;^|v?J!wlcw{{2ov3&<4Tn;ALP%aaZ6c&)wE;ARFCY=@}FM&c4XkKA06FN3s0 zfCJZMuJqr->mt=u*jKSwyR&7*!MLqo^6FBYz~yf2b{xGc