From 9d6448157175d8e03a42d6942d4c058b93daf42b Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Sat, 21 Nov 2020 13:12:31 -0500 Subject: [PATCH] Handle negative numbers in QIntC::range_check (fuzz issue 26994) --- ChangeLog | 5 +++++ fuzz/qpdf_extra/26994.fuzz | Bin 0 -> 40088 bytes include/qpdf/QIntC.hh | 14 ++++++++++++ libtests/qintc.cc | 40 +++++++++++++++++++++++++++++++++ libtests/qtest/qintc/qintc.out | 13 +++++++++++ 5 files changed, 72 insertions(+) create mode 100644 fuzz/qpdf_extra/26994.fuzz diff --git a/ChangeLog b/ChangeLog index b6cd508a..b7571235 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2020-11-21 Jay Berkenbilt + + * Fix QIntC::range_check to handle negative numbers properly (fuzz + issue 26994). + 2020-11-11 Jay Berkenbilt * Treat a direct page object as a runtime error rather than a diff --git a/fuzz/qpdf_extra/26994.fuzz b/fuzz/qpdf_extra/26994.fuzz new file mode 100644 index 0000000000000000000000000000000000000000..ce2860bb89a11b464da82e120b027f3a51e2895a GIT binary patch literal 40088 zcmeHQTWljsTApPu94su3*nQX=(9UJ-Tx@rBxBC)1&i(Erj&pM^PTaQJjuXdDZ0DRg zBLqhX7LEij%L@-ktOPvl+iJB4u@9?*gtQlgc41+Gm(lJbq*!dT}|7B7%SPr~mT(ANqYLCNM11C|vpz3C15yRqLAC ztUlLD41!;`859%Y?^Wi6K}b*-P869OF2`_^APVrWB}U*75C7O=cozQJV>p2mV~jtU zWc*sagsWyA`SYx)mHk*l4wPYyA0#s5UrgQaGenYv+%hDf7$p-qiIPc(rDO^Vpx~eT zcW|sAQ!)-@$Vn^(@azlzx27qdtkH>yvriDz>!~enP`)V?n3(+kV*CeinD--3-}ST7eQ7fi=+|`mjOG+?}VClVVEaU zy*lINeooiLDg-rQom}-Hk&P+jKX|iFU0bjZtPSVV5xNY-Ir4u|j6n;ufuTtHb??0! zjfpO(ZMD{@K-c2`-)xjXw%^jW@7gt>nij~df1((SPV(rc)>cdKAk+rNg+=)72ALoU z%M9qD*+_^nz&c3b7~m@oOfRoi?b84IfBuc{{~;=V>JOgyzW>SpZ2a6` z{qYx*Pd@gkCkh(dY7+|E`YWTg8h6bi^a(bnWvER52Li}|9(2f}>1Lz2tHE=UKAdsK zMD?!59gKguxVtVeGO!y_VWHW$@zx;IXhZivF-PwbBd`XRUbAH)e(2g}3u|pwpHj)M zW`h?Rcc3>+S$2aVg}tl9`c~UDjR~!_{tb|0!bv7J#YD45$4vdMR=fXW%m$fhr~2D4 zZ*6JMuvVrThL)m^8%&o(OVJz}iy9~mGpcUf9xuZ^|f#Ie*kp+3F}4Y~Y~09=5esqg5?}34o%8s+_Ls0fSi9rmxkVhQ6jk zroN_G7`<(_&d`^;*B4L07-E2fyn}+*F}XO#fZbqidh4Bz^?uNDtrqBAPw>H0ve^R6 zC-`@x1?I1M3$R0I0B$ybei_vi2Ss(%6ulF;*Ehz*-%!eEjRyjhQn;sHXTDCmFf2oU zEyAYJ8a84YfuW<}1a+b#ezHp)bt_Oh*3qy6({4>2bzD2q(MC&PLk~^5I`v^ryPtHM zzMo{$KX{klW4WET);|9AzxloQzcU*c`Q&$g^wxKxYm?tS9GF|jT{$o}>{(hq-lNQo z{lVT_-~DOR|C672i+%FqbH2}dIbwK9w}$1s+R})p6kXM7?`Z96Q3aA*s)NT_tyh@n zVYNP2Z&fV;I11`D=(&u!boZ{%CO$G|MJ&+Sk<{my{c$8l-U;ks`F_Oz!it>0lMvK{ z4~%^ff6gjlSJd(i%^+-3>z8M!m`+ibo*~xU0RBO)(cPCWnTGv)d)E*7K1X-G8Fkn? z!Qoz55S!CdA0ak-xl4#`ZpVWV>wZDE5dWPW;tAj9X^2f-xp|z%^BMUF? z+4oBxIGdDZw`Y?^TsWIp0$O)_`fJgToDAXPh&4zb1R6e*J_2Y~u$%Nt$+t!$ zO=>3GasQEkxA{UguY)%UvX3TB98bL5*riDm-8+10lWLC3_IvJ3(5| zDY9#%?a|@O6W=C{G^rV@U`N_y6sJuKrakr3rpU!eqh>^VeE72D z+wwr#BrrSDCZo8J*8QKp^0XB?b&Ir(5FbWb@5-)i+8!Xjy!LH-AdPEw+qB6jPNeZ} zqki)=38@}2BNE*^VGm;;OxH-;BgB{Q_|i1eq-Gb=;96VGr_J{OtM5P3CWCcp(?s`< zkk((&Ez;j`_|w1c+wnl!BrqF$I5&P-E7D{u4z{O0@-+9z8PUFt=t1@{HfVK?w8NkN z9bd)+X_LV0NSln}Vh`N`>MzodoDsV??5RM@T;|M&ukez3vsch^9wpN7`f*r%l6t>Av%{wIa|ZPZQlcLRzP% zu5H>8Bkp=dF0f6F@;yY8QCvuyBe#z{ZS0=v-llDY_#jUk`#-xz+7Tn3dPOdzX585} zneh zwY|f(Xlmd1Y@=yoLvvpzL~H4#OY$JPcS0Wc1V#6&(=6u*5$j%o3#r*{(k7ue$wT@G zgotEC<`IDl(Y+(2b%yMkJRBk7yIz5dWb&7tJiJ3h`TsIRk*iDgAi8&i^y5OruRGl754-{w*>vb^HZ7Zr zPEPia&B>^bu7_ai&@Iw7LVR#Sly!>imOZ}iaHqfQ+oeN9QnTBpO-8ZJ(+`UnYa1Ov)pPH|>TWPJi(73*7 z)bVK<+1g?%>rtq^opSs*cf=M#jr`N_il7#YQ_F`}TdBL_>Us)glq_1=A45ymL-W_i z!Kp-1y%;?>QqPwrbF=ZAk*!p8(iecA^FMv)r?GT0xy8@!*AIU!=>d^RrZ6z@|6;Vr zc1zb7=?z2*O>YFESRL)(JPju&@>jEJVS6~ScOaHio8fp$sNAk!=4OKHliKJ)U}qx9 zA#Qjfd$~GxQ%OA!1jaVcd{TRAe`a>5u)%sEK;DU4D=GH~*oe=x-5w!8$Chs%0W<2f zBVdx0F-%V2a9ocNpxrit!1+k#`fzyX_OfvytIgE5SV+uP(PellP!SJ9DYnH;$tTJ6 z#pG5aT+gh`UKB3xZtsRe!J%mh50jzQtIP%WwA|iaEPC33Nx^s|RE?3~!C^f@fhI+Q zLVTpWF+Xw{E#w;2q4DK-zIuLn9jYf9p#`ONky$D4Z=v(vokadNVB@a%Io< zn~VtVui1Wm7hYc|5wWl2#PRY5;lxILVph$rWhd3f33tBNm~5YHqa1rVtWE5*QuaJl znwc6M-wSM)+D9Tg)^4ZCcv!sJ9ZiPfnQ?Z@3kUthxXrey0pa7}=DxDtG9zNWsnN?6 z!lkhwI<=X?-ptYA$=SsIoVR3kKEguiekBuH3XZYi#lZ%PDQZn6G z-#OYDzKG;5juXW!v4vD>_2_&d5ZEb3BOBgOSoC;L3yAF-KO)wfAP-;ZO>K>2%jxsl z)%4urbo^;;L0KxMYOSYvw5rY?Ts;rOwr7DM6V3GSUi>KUje$vx5SHFkL*KX&vEJl( zWOJ9qWpQ=-xUjN&D#Y_QSApqR2+ScndXN>XLpz7Jxm$j>s?5&pW#^+;vDHY+_dF0u z+|AEavom)GHN_hTlOiENv~aMW3Ft91K*|HJN#ZDWeX?~(Cd=A+nQw(p#*XTHjch$0 zp5MIU&Ubbig^Tf_V*E0vw6B+sH`m8z_O6 z5{_b-2kfnF{5_8^ka0Mi3NSD`Yyb>e?(%Fu6`QS9g{`Z?YGx)pUl`f|&#tgiT;06N zL~q6YP`ZiEHp;d0gA%%}%2{j4Tw@aDJw|b31u96}gDC(qjNb zAh4YcWE*4SL(xmmU<#2JQ$X~OB#}2sLdZEZ5yu1LlvqTNjjK26+~VXSSG+rVy0U#e zeXue!Iu|?E4l>DfvV6OItF3JZF4_8M0Ru4{$qe7#%|#O2k!MVW$crf;`p1;$6-;?P zVp_yWwXiXLZxLg`-KSbDS_`h7wuUd_x2vPeqx_XJk&LtgSuGhK2_E^LuS*xBt3!=e zwD2mXB=03S*qGAm#6XWmhQ}EMGBVz{L0D%L?9Qb9kV22A4C5e#2Uo&M1K~=Q%kOT7 zkFT`SsgRgQm@7vMmnV@i<@9)B;WTiua*`WfS_c;8y&VDHa1~Gp#eLPaFon0T? zt$I?Wb%?<}bz0{NdOv62R8g*vBrdAFUc5Y=4_C^$NkWx)VC*aBYJGN2(sqv)rgr$v zgjBu}n=9;@B+J9wiF>Mix?ei240!`#`IGkxPl#+s!Na8wK)iw~9So65?+A)5kFk78uhR=XnlL=hLm1}phJ8x% zVz&ojKQGba34?zvQNAIclDwD#qJKUm{|cgf<1PewF$F~bn3C_4Pf6ZOaIpE5_EQ%< z9vQ~T4!bL5OOa;FIHf1`8f41SGrQptOg3eZ{Bt;&%OF$M>vECl-{j5Qc_JSB7(z1( zmA?73lPpv=)-aRk7#RbhIxD@O-F72`Oj*{RN$|!xLya3GW`ke`a>#XSQM8FR*)4s~@~J?ICO5@&00vDeK`a z*tFx`3B>lT?k@(Jvg~Vgo`!5>5a`yD?PsB13^HXo)ml@I@mocrEl@*_$r=3 zrmS9FdW7(OaQb(k?(+MR1CYj=viZT!)Abl+$}(oeL8h!;@$?{5R?m=r(jXns*_NBkSWUy5eJ#Fy7fJ{N6v_YOj+F`J;;>RE1n)?%IX=?kDL+hdo&)L z5eJ#F41ao%DXTa4;-W#OtX`4FAX8S)kbdMYnn9+lZk@bArmUXXw6&$XyQMsm<*SpW zyg{a{k7TB--khMdE}^iWptY`ruzeTp(aA95(s%pbf%}7kZ}0>yc`tSR1$+IjA=E1z z=!0Z|DRqS=c8)*iO)rY<+zK;uQzO?ty?a_HW<>t>ATnR=|r$7ze?AkWw%c zmpnLJQpblz9zaO_XWkD@Ds(P{u1{8Sl?g3+aUM9YEVQ?$+49&(dbGG!m5)p1V(ck@ zc`lDgmB?%=f3&f`zO^!+KD}vVrqAPxwO~55=L<>mjmFv8?v#W=n_Ic#oosYsp>P+e zZ4BK=0$lODH`7{(UT(}XJmJ&*_KD{MgCN|}rnUi5@sB{}dNw!{&hn~iF_Qk^fVsNZD zd+JldrPIZ^>EbcFldP<5j*P^MC2o86sT$8tZq?7##LPiBeXuk-vs=zpORM5x=JDJcRT5U8mzqqDeNzv7(*9U>C>&5+s zkd4dn+(9xO%B>+aQw(jOvpX&6t1L%D6D#UQW8}83r4ARHJHzqV>2BzBa-zGR2$pjfOm!u>oN~~ zhxS^%q&?G03>-jur&`o9EAtEompve2W^gd+{$g$G0uC@doQL@iGDE`Zn?JfyEwvzf z6f!P@&=c9{OBniaGWKy8xm}DKjEr;^#&;Iu48Px4I5kc#8!5w$wC z+ys(g$Tbz9bxdD>B^mxRjHoL^a-(M$lDHfG(o=CkLQMSa64D0Zzu@3D`Ag?RKR)fC zUr(M^D_Vgw~Rw)=c*ulW3`sFZ?C;k~xjnhrue!I`AbmuQyCQyI8u{n>S` z-kO5nrXl@U`(`Q{efQmWBk%H&Mzaz{ilRhWF3NFXC=qVGS8uD&!u8g0&}TeY&05dZ zTM-qKbZU`e<2qW+Mp1-~2H{lkb)7$`<9H167gRE9eEs8wtV zB(Eu&gentIY|7}jB<{2@LSYu|ArZQ8iuBwR%O;}sV&-G^zH~i{lqu4i*0YYX(8?%= z)Yi=ZMbn?DH#&SKLQ^2|Zn{;JG#!4!O|O>U!`A3(fur@Vk{AFiB8}2K9(FX}RnmOe zv|*3O>319UPRe?try~kRn)Kb9I}D-eU=wT@4<_dwsLz^O+0O!D35=h$ei$(pgM;d2 zzfTtlEKXI2f0()@<1mT2WIT3j-7$5JIr~v=CWD-un55YpV=*JU zWtiPuG7c{|xMTw6WVZ}|OL56WOu8-^rWi_wPi;BNi7Zb0=#nAK>~0y|4w1(BsN|Z5^y2{CT3UJCK7H770$vB(_%q_!HlS?LGZ`LJ?;rYcaqx)Qv zaV|htIR!78x@CA~bIN3dm-AgR9^bj`mf_taE?JE34_TzKmqk1?x}KF43O|L#d4Qbd z6c#VvxMbMJaLahSh~O$G;Pi|xnTRuUxnvUdZQU|@yeSHv=pE(oMD4}}WYk!I+?~`o zN34KvtZ>yw+bLvv#F;-`N3Ol48_cizE>z@^rRWk|~Zwl1$A@iqM)@Ed%TYWIG75W+ z6)2q{KBTbcSc%dZWEA!sR05TlhJ9LK21gP-_UMz(@P&PF$fQ*_iIV4k@12PJG zj$6904+FUIm%r?n{n?j&rvcxZOQgIi91j=XtxV-BhbJtqCWB{n6d( zb9^6$0IMU5CkdcLE>@Py5-%aGEOHvJlto1=NOHMQLb8@j0P#s|Nl+z`mjtA$yatNJ iAY}$8N}5#UlQ~#8@VDA(vrR$|80!Af(WUKW|Nj7e?>$NY literal 0 HcmV?d00001 diff --git a/include/qpdf/QIntC.hh b/include/qpdf/QIntC.hh index 5f7f21bb..e3ea0a28 100644 --- a/include/qpdf/QIntC.hh +++ b/include/qpdf/QIntC.hh @@ -226,6 +226,11 @@ namespace QIntC // QIntC = qpdf Integer Conversion template void range_check(T const& cur, T const& delta) { + if ((delta > 0) != (cur > 0)) + { + return; + } + if ((delta > 0) && ((std::numeric_limits::max() - cur) < delta)) { @@ -235,6 +240,15 @@ namespace QIntC // QIntC = qpdf Integer Conversion << " would cause an integer overflow"; throw std::range_error(msg.str()); } + else if ((delta < 0) && + ((std::numeric_limits::min() - cur) > delta)) + { + std::ostringstream msg; + msg.imbue(std::locale::classic()); + msg << "adding " << delta << " to " << cur + << " would cause an integer underflow"; + throw std::range_error(msg.str()); + } } }; diff --git a/libtests/qintc.cc b/libtests/qintc.cc index 6b35e837..32c3713f 100644 --- a/libtests/qintc.cc +++ b/libtests/qintc.cc @@ -25,6 +25,29 @@ static void try_convert_real( std::cout << ((passed == exp_pass) ? " PASSED" : " FAILED") << std::endl; } +#define try_range_check(exp_pass, a, b) \ + try_range_check_real(#a " + " #b, exp_pass, a, b) + +template +static void try_range_check_real( + char const* description, bool exp_pass, + T const& a, T const& b) +{ + bool passed = false; + try + { + QIntC::range_check(a, b); + std::cout << description << ": okay"; + passed = true; + } + catch (std::range_error& e) + { + std::cout << description << ": " << e.what(); + passed = false; + } + std::cout << ((passed == exp_pass) ? " PASSED" : " FAILED") << std::endl; +} + int main() { uint32_t u1 = 3141592653U; // Too big for signed type @@ -56,5 +79,22 @@ int main() try_convert(true, QIntC::to_uchar, c2); try_convert(true, QIntC::to_char, c2); + auto constexpr max_ll = std::numeric_limits::max(); + auto constexpr max_ull = std::numeric_limits::max(); + auto constexpr min_ll = std::numeric_limits::min(); + auto constexpr max_sc = std::numeric_limits::max(); + try_range_check(true, 1, 2); + try_range_check(true, -1, 2); + try_range_check(true, -100, -200); + try_range_check(true, max_ll, 0LL); + try_range_check(false, max_ll, 1LL); + try_range_check(true, max_ll, 0LL); + try_range_check(false, max_ll, 1LL); + try_range_check(true, max_ull, 0ULL); + try_range_check(false, max_ull, 1ULL); + try_range_check(true, min_ll, 0LL); + try_range_check(false, min_ll, -1LL); + try_range_check(false, max_sc, max_sc); + try_range_check(true, '!', '#'); return 0; } diff --git a/libtests/qtest/qintc/qintc.out b/libtests/qtest/qintc/qintc.out index 2a2ff9f5..5520c635 100644 --- a/libtests/qtest/qintc/qintc.out +++ b/libtests/qtest/qintc/qintc.out @@ -13,3 +13,16 @@ QIntC::to_uchar(i2): 81 Q PASSED QIntC::to_uchar(c1): integer out of range converting ÷ from a 1-byte signed type to a 1-byte unsigned type PASSED QIntC::to_uchar(c2): W W PASSED QIntC::to_char(c2): W W PASSED +1 + 2: okay PASSED +-1 + 2: okay PASSED +-100 + -200: okay PASSED +max_ll + 0LL: okay PASSED +max_ll + 1LL: adding 1 to 9223372036854775807 would cause an integer overflow PASSED +max_ll + 0LL: okay PASSED +max_ll + 1LL: adding 1 to 9223372036854775807 would cause an integer overflow PASSED +max_ull + 0ULL: okay PASSED +max_ull + 1ULL: adding 1 to 18446744073709551615 would cause an integer overflow PASSED +min_ll + 0LL: okay PASSED +min_ll + -1LL: adding -1 to -9223372036854775808 would cause an integer underflow PASSED +max_sc + max_sc: adding  to  would cause an integer overflow PASSED +'!' + '#': okay PASSED