From 5bf5b505a9f2af82ce42fe5c0a0af993b592c7f8 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:56:14 -0700 Subject: [PATCH] Throw When Ods Reader Encounters Invalid Xml `simplexml_load...` can return false, and callers check for it. `dom->loadXML` can do likewise but, till now, it hasn't been checked. Only user of `dom->loadXML` is Ods Reader. Change it to throw if Xml is invalid. --- src/PhpSpreadsheet/Reader/Ods.php | 45 ++++++++++++------ .../Reader/Ods/BadXmlTest.php | 41 ++++++++++++++++ tests/data/Reader/Ods/badcontent.ods | Bin 0 -> 3411 bytes tests/data/Reader/Ods/badsettings.ods | Bin 0 -> 3398 bytes tests/data/Reader/Ods/badstyle.ods | Bin 0 -> 3409 bytes 5 files changed, 71 insertions(+), 15 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Ods/BadXmlTest.php create mode 100644 tests/data/Reader/Ods/badcontent.ods create mode 100644 tests/data/Reader/Ods/badsettings.ods create mode 100644 tests/data/Reader/Ods/badstyle.ods diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index 5fa253b1e..08fb4f19a 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -339,11 +339,7 @@ class Ods extends BaseReader // Styles $this->allStyles = $this->numberFormats = []; - $dom = new DOMDocument('1.01', 'UTF-8'); - $dom->loadXML( - $this->getSecurityScannerOrThrow() - ->scan($zip->getFromName('styles.xml')) - ); + $dom = $this->loadDom('styles.xml', $zip); $officeNs = (string) $dom->lookupNamespaceUri('office'); $styleNs = (string) $dom->lookupNamespaceUri('style'); $fontNs = (string) $dom->lookupNamespaceUri('fo'); @@ -409,11 +405,7 @@ class Ods extends BaseReader // Main Content - $dom = new DOMDocument('1.01', 'UTF-8'); - $dom->loadXML( - $this->getSecurityScannerOrThrow() - ->scan($zip->getFromName(self::INITIAL_FILE)) - ); + $dom = $this->loadDom(self::INITIAL_FILE, $zip); $pageSettings->readStyleCrossReferences($dom); @@ -1428,11 +1420,7 @@ class Ods extends BaseReader private function processSettings(ZipArchive $zip, Spreadsheet $spreadsheet): void { - $dom = new DOMDocument('1.01', 'UTF-8'); - $dom->loadXML( - $this->getSecurityScannerOrThrow() - ->scan($zip->getFromName('settings.xml')) - ); + $dom = $this->loadDom('settings.xml', $zip); $configNs = (string) $dom->lookupNamespaceUri('config'); $officeNs = (string) $dom->lookupNamespaceUri('office'); $settings = $dom->getElementsByTagNameNS($officeNs, 'settings') @@ -1869,4 +1857,31 @@ class Ods extends BaseReader } } } + + private function loadDom(string $file, ZipArchive $zip): DOMDocument + { + $dom = new DOMDocument('1.01', 'UTF-8'); + $orig = false; + + try { + $orig = libxml_use_internal_errors(true); + $result = $dom->loadXML( + $this->getSecurityScannerOrThrow() + ->scan($zip->getFromName($file)) + ); + if ($result === false) { + $fatal = false; + foreach (libxml_get_errors() as $err) { + if ($err->level === LIBXML_ERR_FATAL) { + throw new Exception($err->message); + } + } + } + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($orig); + } + + return $dom; + } } diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/BadXmlTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/BadXmlTest.php new file mode 100644 index 000000000..0964d733b --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/BadXmlTest.php @@ -0,0 +1,41 @@ +expectException(ReaderException::class); + $this->expectExceptionMessage('Opening and ending tag mismatch'); + $fileName = self::DIRECTORY . 'badstyle.ods'; + $reader = new OdsReader(); + $reader->load($fileName); + } + + public function testBadContent(): void + { + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Opening and ending tag mismatch'); + $fileName = self::DIRECTORY . 'badcontent.ods'; + $reader = new OdsReader(); + $reader->load($fileName); + } + + public function testBadSettings(): void + { + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Premature end of data'); + $fileName = self::DIRECTORY . 'badsettings.ods'; + $reader = new OdsReader(); + $reader->load($fileName); + } +} diff --git a/tests/data/Reader/Ods/badcontent.ods b/tests/data/Reader/Ods/badcontent.ods new file mode 100644 index 0000000000000000000000000000000000000000..b9d4ca0436387c4245dc867830658c53654611dd GIT binary patch literal 3411 zcmZ`+2{=^UAHH@fvWz8J##Tv#MA_FYV~i|=hHInjf-+O79TyyjzssI zYtE$s5KCS>B-2E;W2{TIdPXQrARy^M$$F0T;@Z@^7{0hUNt;=N<}$|n(d7kAR%0&> zHj36(hvb1;F8eu^BVOW5kaCjpZNjrSh@Ngu4<$WekYdIFopWwzJTg+H!F0Fue4qN% zqxO*x4X*XCX-Z-HQk2etKH;8`^<;6arh2BMD&PYsbJA>#Q}Ch{PMF6*jN35dC4m|A zkAEwtOUvW*qOZ&{1t!{z&FMAK(rjFxIh9+PaMaWmfDI13q}@#wem!wXd4_0)OtW6&%ez`+dc(6<`8@8E z=cDH_V0MpwQ6uZlliq>l$r)lW&(8h&kZ0*L#MGwHs16OUfTV)UsYk@TgyO`b8n635 zMd)>-K75`aJA_wD-$KNf*KFSv6k8EiDQMvKE|?TdP?}o=`I|u$+fEC7WJ0 zID(yr=xa`-+hCw)f*vMSh{zw%ifrh~Z*SS7CKmJ{{NwuwSm|e`ITcmY@wI_lJ!j~YE%v{6A)yS)1EunK)cL_T=W9;Z_{$U~M%1 za}Z)$GnMGUHzj@HP>pixt%-75&C@SKDk@#)Y|U)1y%WW}J*r_d;Bw9THGW1TkZPgP zU6@rmSE)N|p_O8tA}3{WG;Gz3=k0pKJ?1X)oOm+1ROmz1vG}yZ@vz3$mB*L5TovX; zibc)&9sCgb$NAbZDC*ZQB2Or>_DX#^QKEIpf&)y&C2og_w-wB7&?ta${7q14US6kp z2*%bL4tR&7>J9~X<@i&vrCDahM>XawPD%M{c{N?4a&m9RAMB?!^bC+1MSP=U%Te{o+v9a$~L_WZXgXSX17|Hn$0%o=o3C7P75e#q)F#Wc<5gZP#=QOCF(2-G+FQ%(aeZw4x#E zW8{;31?6q9bJM1WqlGv=N}qSJBVKN^_jfOOBGwu8n1Zh(PM9g%^jygFI9rrHDbFkE zD4E+1pc6!_o;Uhhp9;p>^|sfBCqM@sL}a<#+MzTRF6kWLlLK#rRqNVaaeS-D>VTHVErur4;e;K`kY+^* zijU&5si8c_BJk*~=6#W3mq+*R3uPATlF&-t5<^Xf5)M!sNT((4w_5lp>}Wg3>;wfz zC}<8!3AZq1xYKf&PD$NPSGnNpFOR4>m=eK$TnGgjhI3*gwtN4+z952KGSkDO$&9_h zndno?qPVVoJyq_4o7R-$Bo#uF53i--#V32nguZtobK?q&ZvLF43AN#dmNcziXKdsS zb=yzjD@V{*2SM0p zLwir0i!OJ_yt{WMtI(OXF`Yj)i3%#D;Q$9E<%KYFt@=OYOJ zP)zJs*B;H_@cVdI?QPd?!G`eCgzD*n%}Ez*cDi6=klrpo#hQ>bh8+TfUM#MIUe#wa zd6FOM)yQFbt23dAsaik;Ulc!{B>uTGJIv6)qi5dUV>D#m)RF@pg(E9A?*_2oZGu9$XRjAcL5vQ zO_q%%^}vcvv0?rfhQwp-_X`K3Ora;uRhn-0-RRwc2}BLZlcAsz!M!#ydsqX&X3)0#b?yIU|C%8q2 zh#m2fTrcuh^u;aMgx#sUm%UwlyBBC&p3k$Gp1jc$m~iz!rTvw4q2lnJmfOZ zOh{F;Ywoye)*Tg`y^4Be(RDmY;;+Hz4-}BInU3NXuMx1*=9IYXG0;iLg(pIuT1Jpf zSOl+(KR(cwwvD5)?M`xpyV|W|borO3-`|srrl;B81NrR)XK4xJ0Kn|b&j{{^x}(Ca zpjWmRRB;&|=f3zo1UcG*IQkRn$Kfe#4^+aoce6gF)zVn#MghtO0DHcj_1^yO$X_Qt zA*BUdpvWD$NCWW%`iitZB;_WYx7AWfvsq_wJi(1PC^e+JxppN}7rrM^!NjjA6s0K^ z7}sUjc~aEGCR#n_{k;63F5K{ZA5vf|Fi`JI_sYymOOTZHO}Vt*q#jnK5{t zs-m(w5y8&y%x@5(i=akM&&tQhmCeQD6}5jJfBpaAQQJgipiHO>!R)nm&+*d zx)m12S?tk3SbF94kF>JR+fCU%NGRI@N8$X}PpmQ0NAT(MgH8KzAFrUc`chg(`cmYO z#e$M}T6@ciEp(Y`3-xk-8{PCEYmra;U;!=5xp~uc`Jc0?*(tZ)lyZzt>e2!vh{>h= z`_e{=>c{3o#>dABXiREl)rXd)AoIm`T01Xzg(X&# zO?b8`8MAXGWdYq;<<2%&M7ymfHzeJ9pp~^XlRdc)Z){J^K>{6jn0)^s^_a>8$i5hI zcE`NZPMg$w+DI8qrWluaN`qQs@i#1V)hvps7ln+7{W|Es70VsFhp4KeQLwQDUT&$o z`F)u#&jplYIyvK|q+XV7W`Z)1!pDJ$9pDqVn44(z3MX7JVBs zFfy_Jm-YVcFzFrm$KYR$_@5Nt=KkNE{y=`G0Mljczr5<7EW3T`?<|gVmfv0LH>3_g zU&6myD(N3kF#Y-Mf8sZTwdcpW7yuz4(Rj5Osxv~T-rfHEtlet-b1s-Z;G`GXPi_7a vwY$83M=i6{Q9p|PPt@-6{T*e(K@Y*NVzz*>uzq{XM!$&k75INMlED7}n&iFT literal 0 HcmV?d00001 diff --git a/tests/data/Reader/Ods/badsettings.ods b/tests/data/Reader/Ods/badsettings.ods new file mode 100644 index 0000000000000000000000000000000000000000..1de2e0affe98d7c9018240e9a13b4763b47dbf4f GIT binary patch literal 3398 zcmZ`*2{=@38$Mwgd-i0>UNp8ujWtYYXe0*7SZkDh8Oai2EZMj0OQNjVB9V|}-}eb) z$(HOY)G+?hRo|bk?>px@=e*}S?|onIxu5rWo?AzYoPq@aC@Fy$^;0HwR-D_5001nJ z0RSsN0U*?nDq?VLbqRY52Wu-d##P+i-cHW|0E%=QY#JqLYu6zDYh|y}gRJ|F>oW;} z*fl1sSn6ewuE`#eibZxmHqW@{x!W0nYg-FbVa$=s{Q93^^@U{lAw}7Bx|1!i@6Wcr zjfn%*^hV1v!Y(`;Vnu_}u@P@0#Z;6l`*BIpqqr+a;LB!jYK8GHDexYQ1)ddvihXT5W$Yj60{<{-VUErMg$6$o;Mlobi`9yYcDylb$O%jZjvug zB@v`^NFS0k07;sk2Rlh0ETo2QYxMILw=*Z_)JB2y7kGhHSQ`fG(vU!|z<{ zWx}N%Hz<|~kG?++c~JtD3uwGvl! ztgR_jkR7Su*%vMP1_NJF3ulP!PFFsO%*M~D1xXeY%N}XA9)FT_erX5FiFKxnxuCzV zv(Qu5?c;k}>J5X`?08ji@TGFG^BJZ_DKcwjFJEqtu%S+FeZ{`=(`|lP=$m*o%j|IM z9`@o<+vEU1%>)26q}bhbbZ|vG{IjE1t|Owx&NI}ml{m$pOH9C%88x^?=LmbYnx=f6 z(bRi{2w{%V;3V^00-ydrd~(#|$mPJTH?!ifIy2kgx?RGd{_ zd82<`nHOKMoMVJ?AjOcDnrMBaCB*%kUB>ew{S(cI0n~*7+k8W4ZFP0S#3*x)qV~`Y zcck`N=1zo_lV|!zdSfZApiK-eNmoBnD+jCv6ZNn*jJG<vMRgE8^>v!k4KTWb%BY9y+C^PN;0@9Knlc(!cMb0}s!pW+H6C1t*Z7H*d2pMJ~6M zsJn$she4M!R&OZ~8G^Sd@u+|cDaJVh&|1peOTDJfMj1?WP9jGsEIAW5_xM~?lg`mI zb9E}>W^coMy$L&1CgrhQZ+C~x-$QP~@9L=6AfPqAqX{4H zFFFm=zwY<&VWM`;VQ;78vaFbXUVRT~EV0l?x0R{nEX#}+Su7*Wl@?j1)WkDPGaZRP zdI>uleJ1Qp)Ka1NtHyWL0nr}L7e1ypszfA@>TxLSamjPKtbnLL`Q+pkX~(LKz#5a! zR`CdDCT98T1i7fI-Qs;2@3bYWRwtLzB#MOZa|Nz}M5=LJakY^ay<&ZI&+y7r-{P&G zwuU}2-wj}W%65Ly($n{;`N|dJ?oMIpnlw(KhhZhQ34jdOHE7f6OSyJwY5Wg`nX}4; zv+(-w>QT4m(kyKnDxDpjrWhITgZL$$fvR6m-3U70YXmX!AQGxduR6b1wL(%;aW75a z_Jtho)Sva+7oe;d+%=cn9J#PF6JXN&f4^ZL^ZLeb4k>n#Vd6Xv^7FbBe!GWc#jaDffP%l8A>$T8J8q#wX_` zsH7^;V=L?;!QdW}0ny!I+(nnj80zuDS$mJD$6L;cG|S^uD{# zedq6|zqyyQaOSHK8{`M?0Dgv{gL9Af#ioza`Wqh;NNC{ZLrl0N;9X|;qeI(9!woER zhvyNFKk73_h=V?3L>%#h&j@jxI7*x#4iWo_!$iE2W4+`4gC3rVterStaUZ$ehbG>0 zv7h!jpR{G7GbPeDruBEYyvMygzNUnQYRLIV!Xu|RXzD%iVgvdn_Y1~nvV$Jrtaa8|G>T$Tkr>=BGpK#O97SqL-z1Wp=x}byr*7Y7Xq$yO;y`! z!L{P`snnQO&qdaXnwfPB<=p^zGWQDbLf^>1O+lrb`B13C90T{*`>v=U`sS=JsysSi z4!d=xJFXUPeX9g|=cY9BSM4fnbCw<#ly;AO@C`p}KbM8t?yT?-t(3_5N*U4Ei5k%%FH z4+lBWPBv7O)PTiF*UunZlfvxbgg$86pawrGfR~cuZG_#azTf~|xGixNib-}d-f=Hx zK^d@6jYQW>BjUl&W&DVTS*r-u1Q5v^z`>*zL)tnRR}VY%&*px>8xh~&kS)m&v_@cI z^m5~RS3H;*ZEBz>lc>La1a*!9$tf_V{A0^DRT)7YBXQZi&euxeq*r8*VfT6N%laX5 zVINn-N0kw3rbB49Jufent9@S}3ZzME2U0?Zc}K7R@`e42JW03v{g! z&^+V-Qd%b}YcN;uxXpFvZq+&Zts#zLIi%f(6KGV~a z*@R$s2?$+(<034LNj*1|rVc6H*o0%ClpEU=5L?MHRNQZ7;^6!C=rSV_m;8A-x6rdM zt;kH@mb*`P{yV=-KfJWMa=t&a*q^wtEarvUCwJ=|?AH|X6loqav$JniN}N}zs>Mw_ z!fE6>3$nZm$NK9i%j)E;-(pvPKXTFfrDPGRZ)_dP9jrAO&BP$^IBO`?qKQp9teY-M zK%k}YN2*`y2>ERK$XM20r7u6g)csPJ*h&BFxzc+R8=Ha(d!%i6gpB;?f4S|Srjm5y z{WbVk@BKH$!QB5F(qG7*6uhJ|_Fwk&faM}UTFAdzQ^8*}yrd_J#PX+6rT(?NBLF*~ zU?8+|`68Y)C=V{z+yXEzr?}uVB*op7iT>GzV}01JNwTZ~y=R literal 0 HcmV?d00001 diff --git a/tests/data/Reader/Ods/badstyle.ods b/tests/data/Reader/Ods/badstyle.ods new file mode 100644 index 0000000000000000000000000000000000000000..0a3ca847d65b50733f503e831a5d602240e6e480 GIT binary patch literal 3409 zcmZ`+2UHVT8y$hrLArn_MUW0kla3^U8$u9iN|Po{10)1lK#Fwf9YXI&m1?9TO#wma zAP|Z)krGgfE*SpkarYO`?#!8)H|5@Q=6>_tcSjdSO2z^Jl$1c6=0)>XyR!#t0069$ z001jM2Ea88)WxAXnvzad&W?7rXg3KDCxpHc095F;-|M_Y+kzJjz*j%d3~?MbZOb_X z#BVYMil_e}JTUiGxNeQzpUo?w{K-KUKYo98K7u)VgU|3Iq^*pkB&;I8Rd4PU=T3r1qc6bsKJ2k0&3c6v@+#-alQ6kMQy)&k`THTLZ zY_@LdJ(TjNO_DNYSx3JZR~JTiXsK(>%JOj7>5P7iF!#ahp*a~%&NFDFzKSAa8T9O+ zvudx!6dsWYI6*opcWI6QE&cD8avpe5b@9!emc?Y~Pz3Y^4(4J}rzP60p;8351{ zv1nU2H%DiC^gogsyuA^4iKX@MR_tsp!Yh{ic4S!C73L>H7doa9LCJL>?C~tvs^%v4 zoHFC=!?_q=ABLyy8e5}0{L5lsnzz^Ks}k)Koi6NReO>WGBCS+k?I8L*95>2HFs=eZ zh1U!67qpp1qICt>7s3kSs|o<9ftCjq;!)g=%vzVWZHoRrbFNFFRsd>5<_>xtRoxxI zLy4R?r>Xx&&h24EV(NJq>dk4e_zTQhS8}Ug=zx-|S33uM)boYUG23Md|u=TOY@hV2msoy{I zogSA(k69CJtEMv4V2HC(+g@TI=%8};uWz-dKkT*mF2Hk{py?myR&3xZUmtJJ;01m! z<>Nt|*#Rj4P%{Al4Y3?HF3xVY&i{Pio44UH({cw8%R6N`7dL`2YcDpq>W+)&dvNT7%)OBJKmF;0e=q?Fe@?(6GVrEC^W0Ie&R zJV=6C*Y}F(NhKJkLAy)eh(-*hW{XC$H!%m|ROI)ui2I?6N*i&K#`M|m0&VbS33kP` zodLzw4|zisU81fAla0YN#M(NoMLqTqS>+Xm953M`W>-e;m)w$XX=rGlonkIh(HS%G zFwnWo+z+=ydS$++Hy2>E1jY z_!!ym2H%}m{ye{Ekh;>TZ=3e2RiJuZ`tTw5T)>c;GdK4qtrW*R%!;+1>bQlyIe zDSG<`nGpGntgXAs1cuN9N}O5Hl{C|$3-T?LPp%E#K^bQ;(IJJ&$*j+&eEP=g3dTs& zGjsK;OfK0&{Cp0-P?^`pbGnx0C2 z@4bc`r*9ng^kt%UD`M}XdO1fPW7nNDSb}mZ(K@eT{l70AAF07D|F&DCLo=u zm+QHOa#HdFbVFPJAmN6|frOT5t3mN0xlX@odGq~x%Jt} zV*zJ}w#)v<7bxpSk1Q`8owdvWdkGyX_7&i@IV@)#@?s5gOBz!SEGm1Cy)p-BvMU%L zcIvG(K?eNO8G8rPE;nI<6E;n3#XC4@YuUDnp6-(6`)$Fl4t}PROcK3hC1y00lvd|E=)R@Kro|Z0@rh z|KVGIZf}llj%4P}g5eh~pBW#|-L#1C>xu!<(CB$inl?|I_=usocj?Sxe#oP@M}qb{ zBquFfoavCgiD=n$M9ZcmdW4g$o7LZT9hETWJk7)M5`Vzb)09by8h@&e6?Gb^ONYiK zs{p~wp{#+RdgnRcqcBnX*NJyX+VI}op%9v)Rr9T|9;W33dnbA8i2Vq`6nn$Sm=Zy3hJ6&Ubge%tHWOX;)Bt+EDpQ{o~oS86i8 z%}p@Z=((OTA&oQU9WA6Ga}R`oSB$^%TX|XJjO?AEs|W(Aj4nakeiYe}SY}V8?e{qx)b39O@KBPyh(cT(3JubO-Y0Cyqf?QlUp$_& zm>IE9O~f`Wz>_G-W&H_1S&1C#1%l?~_g{5`h*3eCf;hzxnbB^Z2-_cXeN@sw^fV8{ zE4)v5kBU-Py3y2a=)T_Gs9EVIX_d#u?Pb^_8~n87Jf^!>@x66#KVD6mR=UcMgWcTx z>YC9u#HjqrJ8F4G!`JDMef-{^gp3xhwoMK5=VX0W>>}!lsBQ&VvXV!U+$D@bwT->WQS$kBoz;zc^*30R#*<}} zC-@AN9nvQF1OuRmsdO>&sZYnD8;D3T#*c@-IF|{qZ1Lo#dygrkgCAL=KdR5FezhGm zQhfW2gEa?xMq0MbVxq^Z*{g-6qKIu=i56&z&%G(&iDI~HEum4SL_(}l*Ut5&NW)uY zWtSwUBoTK+x$R`enKuU*eoxTWcf9-{*F8usD4F$|C{(F zUi}U{>9W59PZOKxZ_oW5b+T=LMeR}&Q9qmbJ4y{8>in;5$^R3^Lp*9x{o16|Keayv iumg^l1eU;RIO6M{Ty$X|^6ytDh-(+|?w>T@&;AFQHOOZG literal 0 HcmV?d00001