From 3eb4b066ab3f25f6454214d33b2fc17161812dfa Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Sat, 5 Oct 2013 06:26:06 -0400 Subject: [PATCH] Security: better bounds checks for linearization data The faulty code was only used during explicit checks of linearization data. Those checks are not part of normal reading or writing of PDF files. --- ChangeLog | 7 +++++++ libqpdf/QPDF_linearization.cc | 14 ++++++++++++++ qpdf/qtest/qpdf.test | 12 +++++++++++- qpdf/qtest/qpdf/linearization-bounds-1.out | 6 ++++++ qpdf/qtest/qpdf/linearization-bounds-1.pdf | Bin 0 -> 12302 bytes qpdf/qtest/qpdf/linearization-bounds-2.out | 6 ++++++ qpdf/qtest/qpdf/linearization-bounds-2.pdf | Bin 0 -> 12302 bytes 7 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 qpdf/qtest/qpdf/linearization-bounds-1.out create mode 100644 qpdf/qtest/qpdf/linearization-bounds-1.pdf create mode 100644 qpdf/qtest/qpdf/linearization-bounds-2.out create mode 100644 qpdf/qtest/qpdf/linearization-bounds-2.pdf diff --git a/ChangeLog b/ChangeLog index 7440f632..8a10865f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2013-10-05 Jay Berkenbilt + * Security fix: avoid buffer overrun that could be caused by bogus + data in linearization hint streams. The incorrect code could only + be triggered when checking linearization data, which must be + invoked explicitly. qpdf does not check linearization data when + reading or writing linearized files, but the qpdf --check command + does check linearization data. + * Security fix: properly handle empty strings in QPDF_Name::normalizeName. The empty string is not a valid name and would never be parsed as a name, so there were no known diff --git a/libqpdf/QPDF_linearization.cc b/libqpdf/QPDF_linearization.cc index 2c4fefc0..dd09b1c0 100644 --- a/libqpdf/QPDF_linearization.cc +++ b/libqpdf/QPDF_linearization.cc @@ -295,11 +295,25 @@ QPDF::readLinearizationData() readHPageOffset(BitStream(h_buf, h_size)); int HSi = HS.getIntValue(); + if ((HSi < 0) || (HSi >= h_size)) + { + throw QPDFExc(qpdf_e_damaged_pdf, this->file->getName(), + "linearization hint table", + this->file->getLastOffset(), + "/S (shared object) offset is out of bounds"); + } readHSharedObject(BitStream(h_buf + HSi, h_size - HSi)); if (HO.isInteger()) { int HOi = HO.getIntValue(); + if ((HOi < 0) || (HOi >= h_size)) + { + throw QPDFExc(qpdf_e_damaged_pdf, this->file->getName(), + "linearization hint table", + this->file->getLastOffset(), + "/O (outline) offset is out of bounds"); + } readHGeneric(BitStream(h_buf + HOi, h_size - HOi), this->outline_hints); } diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index a30dd7b4..87c9d8c1 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -199,7 +199,7 @@ $td->runtest("remove page we don't have", show_ntests(); # ---------- $td->notify("--- Miscellaneous Tests ---"); -$n_tests += 67; +$n_tests += 69; $td->runtest("qpdf version", {$td->COMMAND => "qpdf --version"}, @@ -527,6 +527,16 @@ $td->runtest("ignore broken decode parms with no filters", {$td->FILE => "broken-decode-parms-no-filter.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); +$td->runtest("bounds check linearization data 1", + {$td->COMMAND => "qpdf --check linearization-bounds-1.pdf"}, + {$td->FILE => "linearization-bounds-1.out", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("bounds check linearization data 2", + {$td->COMMAND => "qpdf --check linearization-bounds-2.pdf"}, + {$td->FILE => "linearization-bounds-2.out", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); show_ntests(); # ---------- diff --git a/qpdf/qtest/qpdf/linearization-bounds-1.out b/qpdf/qtest/qpdf/linearization-bounds-1.out new file mode 100644 index 00000000..eaeef14c --- /dev/null +++ b/qpdf/qtest/qpdf/linearization-bounds-1.out @@ -0,0 +1,6 @@ +checking linearization-bounds-1.pdf +PDF Version: 1.3 +File is not encrypted +File is linearized +WARNING: linearization-bounds-1.pdf (linearization hint stream: object 62 0, file position 1183): attempting to recover stream length +linearization-bounds-1.pdf (linearization hint table, file position 1183): /S (shared object) offset is out of bounds diff --git a/qpdf/qtest/qpdf/linearization-bounds-1.pdf b/qpdf/qtest/qpdf/linearization-bounds-1.pdf new file mode 100644 index 0000000000000000000000000000000000000000..44befc22123ae91b2802c71d5a47a4df6a9c3001 GIT binary patch literal 12302 zcmc&)-HRkw6;E88_sbq^CP{ z*MwC`a0vtj!Ns?L_~4_@;+tp?!6)$#@WBV41Ybl+h#;PGKYFU}ts3a*Nvfe|x=!DF zzR#~t-#S!NezcOU=qf1Q{TO6G@ecDA=k_s(RNXN$@A^ATZiMHm;9lkRQu zfH1}+VJRmMN%ua91?k=*A{Mah9ubsu`-IUn5oBkllg~!TkbJJM&f`TsMr&Bwq4vch zqCb%taWoZ6pQbJI)-gfjV4aM@y-@`I!`G<{7Dq`)OVWU*z>EIgRC1rjsZs%ri(oI0 z7z@_Lakw|(fzCWl1Daqe&{+r`$PiMMd{r#6$uwUOOsC#_UXU0zkRItCKw1&t*xg^` z?_jbh86-&ewO#UHJ7Y=2WHKDmY!pW$l`9p|BolN<4`V6B&Nh=HHR4i?;!&K9#Nkkk zRWww?C{3l1@o48E=Har)7KKg=+PQM&*8RPX*2N|J{gYw}`MxCRBGhm{JIe25hxv4= zwZC_AyoAK3`eL7KT?L+m>#GCOU4TUeR0+^jWkp(D5#tI74En<{VJj}XnhV!!BDc{H z%|^q`ZG^xf9}sWW03+1NW5;3|rTPoumK$v$S8=(gA!IdB(PPF1cju#=bicXGR}Rvi zSrTBX{nY>7+pyqfR%Fxp5o~h>A4BS{pk!|{Eg%iLd(*7Qck`iUdO%YB0&K}=N5vy| ziAL=I{A10mtXws8~^?7Z+`WI z@4k2Uxi|j$;gcu-dGYDxPk;LESD*RE@$;8aYb(E+A$$zlkJg|XTLKIQ`>G2juf(vDuthX*-S2b7K_ z8Mk5*mVJuSUB+6cNlXpruN!>;+U_1BT$276J}b}*@}xh4&s+QRA}i*EUM2lEJGc5Z zHls?8|BY;!Ykj-7^XWTzF&Sn6^D~IuWOhWl-J)gnQXBBm&6p9-JH&IxEl! zxHh(EUZwm%TT#{nNVMHOSd~96$ir3oU>Q19Zq84@Z!vJ6ESB&A5EX-PC&P9uKoKC~k(}}~Nb4w^uU?GIf#HQ*97phYF zEGNT9^93}5*SQUmiIC6otcaBX)#|1}79tQ$@u$r{n^@Ic0$`YfJGII#-?{Oo7I2me%lK?G z1$cG!XcevZrB&J7U<`6%qgG0Pr=@YTtz+xvipmB@P~}k6S;{-W#~Y#5S)Qr0bD#BQ ztu7`T`TE+F?=MyxVFiU|omPqsvs3L1H@Vm*YdflZ!*Z(GT&q{j)5b*5?x^z`)QY|r zr0c3y1zp<{*@|qZB`O2nxjQ1(*b$X6Yt;nWptoai8LZB2FmE)tiZHJ0H9*mh!DZMu zx51*(;4%)ZB5bR?9fQl|e{O?iqrv6ga%C{HO>R3DYw50;JGaSEqse#_Qfd}$R>N;L zegA98<_kftxY_1f=z`|2O`9(Syi~KzwdlocpVLyDF9g4mW}9mPjJ1kjTKdNIoHpBB zi(z<>iT!Llan{m(o7K8!GpzY=2G3eU5QF2sHjSCt%f#!4tF=(_2e-I=Nwdej!d=+WLo;hM)T&5Ox6M!KC^GH4b{>&H@dka(?_;Wz^bNY zt3aluJDtv*-n_XZleHZgJbG<2(iZ|`ytyNjwLm6Y1u`vt^GG*$WU>~>aIUAln%2@c zH@dkai)w)^Y8A+|^v#WK?#QCrjtr-|T8#9C{IulF9a&ThWH3w6rqLGzWX&B}R10J{ zm)ByXQTpcT-Q1Ds`79^EJS)b@%&$*IILBrmykp(>V2%X;VrKMUd=3912=;JLP5(3B zAUK(LeV@aa4=LR*yMZb(gbR&N`v9a zz>WwedxLcXk9@qn5@#hmT7-iG9!-UQi4uJF;H^t>O0Y`PXWp`UL#I+<>3rNx)A@JZGrtq9Uvlj~0jd2PqlyOY!)(x;LR% znjVdXgTDz6ho2LP&&0gF5e@mpSb5$$73Oo2!oaw{4rV@mTFS84tNc=qUA;#WFk|V{ zWH?Nn#)vQ8UWLaU9xVxLAx(opXGR%9T4gVz98aM=ng}q0!7!I^FM}bnke@2oM|p5s S&q~8A`)4%|%~@ literal 0 HcmV?d00001 diff --git a/qpdf/qtest/qpdf/linearization-bounds-2.out b/qpdf/qtest/qpdf/linearization-bounds-2.out new file mode 100644 index 00000000..bdf7c91b --- /dev/null +++ b/qpdf/qtest/qpdf/linearization-bounds-2.out @@ -0,0 +1,6 @@ +checking linearization-bounds-2.pdf +PDF Version: 1.3 +File is not encrypted +File is linearized +WARNING: linearization-bounds-2.pdf (linearization hint stream: object 62 0, file position 1183): attempting to recover stream length +linearization-bounds-2.pdf (linearization hint table, file position 1183): /S (shared object) offset is out of bounds diff --git a/qpdf/qtest/qpdf/linearization-bounds-2.pdf b/qpdf/qtest/qpdf/linearization-bounds-2.pdf new file mode 100644 index 0000000000000000000000000000000000000000..bdd6177cc213f6b90958aa7490e371a14bb253a6 GIT binary patch literal 12302 zcmc&)&5I;S72kDq4h0K>H&21J>H_=4|PvRfo!GkBkizvISAinn^qB=7ov!T0s&4TK#d=>G& z-!ET9zH9rtdpGz^*}L}J4}S6AUL3NJEe_xAZEv&S-gI8%%jvg@G2?K>xR9Y>!Cm%< zan2RvnP87u@Q}rl1$UT?B`kZuWXOU6<6)LcwzJbK=3`{Y{;#jz(`7M1Yk1ZR&5K8j zKCwBov?Nw8jVyE4aT&(mIu-eQqX_(muTvR3j#8hNW*#j8FS>g(C0v>alfFpH} z=ia(F_V=d3(^+JhN0U79be2+hGNei>zABgbbXF`Grc-~hC|L{}Sf2$4kX8&h2K&q6 zT}+llc?lA{vCAH9=RA$LN=KtGAIH%+DUu`#(_Dt5@Gw?V?rd{4PR2sXaXgOmu{<2f zNfM2cQIus$s(8Hf81ryd=F5`OBJ5qecIV+r2)5aRk0Eu}P_j3jm5>I(-YhSR-C{&c4_KCE81>*@F+VDw zz@^Rsiq^8ok9$vlHX06xhr{8I4u|=0_~9Ra_xl&Fu=q1qUVZ+De}3=9PpRy+C$C(+ zGWqm!`2P2Q_2#qpKk@A4A8h~WlRv!j`S*VOx6i$}H5ooX`s>!ozluNq{HI^~&u@PH z%Wr@EyZ2vu>o4y;d-m^Fo?rdwC*S$(3x7X;`6_C?DoLgREG);h{X` z9rugzG{3cYN+}USS_{Z#g2VlM2`ks`=@+ZT$#MjVg|XTLKKcV^G2jufB1bP5qk{sf z14_q`v|BL=t3E~PHe-#`6s87bH|c(B06-h;aV8WSOyF|@dO@BH#_)M-e^KV;f`!-F z;O*X>L4(bxlI4FZUlpWpaJQJfTa?pL4lqB5*iGk0EcnWF{`!11JtaKIk0B92c5rZV zSW;G?5pb<-AzqdIKwDAPBS^H}Jy=ydE!pFB`d}GaRcUOpkz+6| zX9e4OWA%>}K*Eh37JY#QzJ%-6F-GTVoKY>Sve6U@mDE5+kS9qLGl{ofM>yfBb&%0y zuq@umzX50kx^SzE<#C1@WVuyC$KVFs>a9Y?6?;<%+uB*CJ}5Ma&hUVoWj$y0H8C_Q zzJ8p~$2sFS$zhy>$AIryx}o&CZsJrmBN$lRwRLHNiKw&=CYCr{I4Y3~ZPc}X+mm84 z3ByQ+Va(xOz*~i3l7*pwoAx9LCEStZA&2)E-U{CE9nk4}33~+Wk+2xoi!pqLYy6Dg zN#HAU^Bu@hG4O}|3CK{XSqHo%(4+ul))nO^4wU<;S4^Bv!_=q5;n2CIAxpqQ2%C;g z-4QlamGoInM^6?@XasKx6CxcUm*rUzs{*RgjX)M6;8#Z_wXHfjE+&9=-Jsr3Gs^Jr zywwJ+$Enjynpbkk_y%(6O=wieX(pFUr=WA)5M7Vz;#XmnO* z>TKO-yxFLW&PH*5ZL0Sdqm49ze6vm~#e~_ac7~g5Y}0c)s(Qn+s+nACR?X2yN6_qO z@*30%-3zkws#XVG(-WDB?4~8E0^Wr?BHr2&RWWPb1e&0CV{jF$E^M%9HMov2w(HeE z(T%}X*toF4ven=!4y+?=s=OP6tL1-TgH@}+)!uS#FgHzZHx`q0+ss|qac`Rmf=O98K>-R4I0;-=5(D9)FHUunC|jR3|wMKF@S zaXn}4HaB7z9%N!a+fAHFx@&WC-m@9remH|?tv-msabK54H-Xssjc)J1L?ep9=~kCU zH=)@1jc)J3L?euePGO9sZ=T-meVAy(G0`cGk@U@tZtunDP|pfjH)HSwt;6&t=~gG( zjGaHd+q*G%OxOTqvQr=<>6;tf-j3lDIv>4dr$9#1H#fSyAEN^mE8sM}I|VY?)Q_D% zz1tfy*$8B^Qy?ShHV_xD>GqCHHg;sHQy?Sh8yhX!J2KS>WcbX!yEc@hZ*FvZM@C1s zR=~QZRi{8k(ydMxPH)lPk*UUx3?9998R<&_GSS|VsYW1EodOw2-#pUo9hqtbGMww_ zuBJ))=0>-7WKkoKMV$f}N#ES)_KqxS?8tDctHVfN%1=wt-jPL(KnAk}T^fBkK-S)o zMU6m)b9o&`8l`WZ-t8S3&1YEw`dKkfW`1!x#yK|g;2rC}4|62=7c-*|<7@aAL9ma5 zYV=QkkMomA)GN4NLWfd)M#nVpPcNaL@^Gb>(9b;(qPdu7tkfg}Q7@rJs?W^RV7l4D zl@>&R4BBa3sA%Cz3!=dG@!_l#dV`HaD8*+F&bkbz1nV>!S9EDG_3EvYp~o*) zc-L_FMVLVjEzW#?N#^rw7{G;5ACDiH>+wrO>hVhyo-@>SNg}KhhZg(!2Px_E%kcQO zzBdiAG#wiE2Y*xH4?m|0pNTnpqtNFUW92#P5*EJl2B_9l4T;m}gQ7P8D6bmk#PNUQDTp} X>!Uh2O|#N4%l>iALkk|8iRk|UV4q1m literal 0 HcmV?d00001