From 236c3c52a9b90688b2e57ec503559409e29f33ed Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Mon, 4 Aug 2014 18:17:26 +0000 Subject: [PATCH] Apply suggestions from Uwe Schindler for more secure xml defaults for #54764 and #56164, for xml parsers which support them git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1615720 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/poi/util/SAXHelper.java | 35 +++++++++++++++++- .../poi/xssf/usermodel/TestXSSFBugs.java | 19 ++++++++++ test-data/spreadsheet/54764.xlsx | Bin 0 -> 8016 bytes 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 test-data/spreadsheet/54764.xlsx diff --git a/src/ooxml/java/org/apache/poi/util/SAXHelper.java b/src/ooxml/java/org/apache/poi/util/SAXHelper.java index 6a43292a5..b38b2c2be 100644 --- a/src/ooxml/java/org/apache/poi/util/SAXHelper.java +++ b/src/ooxml/java/org/apache/poi/util/SAXHelper.java @@ -20,6 +20,9 @@ package org.apache.poi.util; import java.io.IOException; import java.io.InputStream; import java.io.StringReader; +import java.lang.reflect.Method; + +import javax.xml.XMLConstants; import org.dom4j.Document; import org.dom4j.DocumentException; @@ -33,20 +36,50 @@ import org.xml.sax.SAXException; * Provides handy methods for working with SAX parsers and readers */ public final class SAXHelper { + private static POILogger logger = POILogFactory.getLogger(SAXHelper.class); + /** * Creates a new SAX Reader, with sensible defaults */ public static SAXReader getSAXReader() { SAXReader xmlReader = new SAXReader(); + xmlReader.setValidation(false); xmlReader.setEntityResolver(new EntityResolver() { public InputSource resolveEntity(String publicId, String systemId) throws SAXException, IOException { return new InputSource(new StringReader("")); } }); + trySetSAXFeature(xmlReader, XMLConstants.FEATURE_SECURE_PROCESSING, true); + trySetXercesSecurityManager(xmlReader); return xmlReader; } - + private static void trySetSAXFeature(SAXReader xmlReader, String feature, boolean enabled) { + try { + xmlReader.setFeature(feature, enabled); + } catch (Exception e) { + logger.log(POILogger.INFO, "SAX Feature unsupported", feature, e); + } + } + private static void trySetXercesSecurityManager(SAXReader xmlReader) { + // Try built-in JVM one first, standalone if not + for (String securityManagerClassName : new String[] { + "com.sun.org.apache.xerces.internal.util.SecurityManager", + "org.apache.xerces.util.SecurityManager" + }) { + try { + Object mgr = Class.forName(securityManagerClassName).newInstance(); + Method setLimit = mgr.getClass().getMethod("setEntityExpansionLimit", Integer.TYPE); + setLimit.invoke(mgr, 4096); + xmlReader.setProperty("http://apache.org/xml/properties/security-manager", mgr); + // Stop once one can be setup without error + return; + } catch (Exception e) { + logger.log(POILogger.INFO, "SAX Security Manager could not be setup", e); + } + } + } + /** * Parses the given stream via the default (sensible) * SAX Reader diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index dd5e0505e..a5420b0b7 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -38,6 +38,7 @@ import java.util.List; import org.apache.poi.EncryptedDocumentException; import org.apache.poi.POIDataSamples; import org.apache.poi.POIXMLDocumentPart; +import org.apache.poi.POIXMLProperties; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.openxml4j.opc.OPCPackage; @@ -1846,6 +1847,24 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { assertEquals("A4", cRef.getCellFormula()); } + @Test + public void bug54764() throws Exception { + OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("54764.xlsx"); + + // Check the core properties - will be found but empty, due + // to the expansion being too much to be considered valid + POIXMLProperties props = new POIXMLProperties(pkg); + assertEquals(null, props.getCoreProperties().getTitle()); + assertEquals(null, props.getCoreProperties().getSubject()); + assertEquals(null, props.getCoreProperties().getDescription()); + + // Now check the spreadsheet itself + // TODO Fix then enable +// XSSFWorkbook wb = new XSSFWorkbook(pkg); +// XSSFSheet s = wb.getSheetAt(0); + // TODO Check + } + /** * .xlsb files are not supported, but we should generate a helpful * error message if given one diff --git a/test-data/spreadsheet/54764.xlsx b/test-data/spreadsheet/54764.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..938245b61ad44d7caa69b16b620370803f256fd9 GIT binary patch literal 8016 zcmeI1bySq!*2f2ikPd0-mhO_2knSE3kS=Kf>CQn~q=rTmP-F;6=@JD*Qd$J0Lpt6; zyg%Wm_vgLu`|mrmo>|XXYo5>AXYccz{oVVhDgY5s0p}lGS3QaEH$Q%m0HgpTXLCCj zb}da50B~O{LiPM|^FRZ@AuIv`fbT!5u;=jr@PKQur>FoGS{yXXuscO~008Z;=h@CF z-ywj4cev-*-ykLNy*4cz=>F?L^+EmhYGX}AxytOOed9!RLwN z4VO&r^b8cL+#a7027m3O+MVSoj>5&eMN{cs5?XcW`3=BC@0={-R6Ns)^WfuZ?{3l^ z`PYmO>K`#_7!3C1f2P36UGm7$4Yz#kM}l%a{}`S1As4VBI_*_m$3`05$H1apZOM`G zTdLo>wt4NWQzthPQd(NsGOPoH86G6Bc83{Is6z_XctesCkeI*Fh0(J^?13!X7F+kep1hM@^L_t%BLw}Z zgxRyOZza+?I6ch{q|g({JEGe;#K;UXSjCn>D=pr7p*P^t zm1966B*h*-)%ZMOykaKi%v7tGMe}xypc7?()BlqO!9883>i$ZbgN|2KY~wD}1=@W! zt|!MD14#%?)tMh690eF$57*lBGX&2bHu(_`FMq2Mv3$plY&?amkU>{G?jV>!tb(Cw zz_z_MRF?{Bvpv3{un$%)oU_;503%KsHdomwx!2xpszs}-D)zIi9kYCZ?cU$|PgAEC%v zUSfLlCqv(o!DG^y+M03nZ-{qh{1q%;d9d*?2_ru>{4#Rk&5!# zne*0kQ`ZRuBY4;m_iOO`#<#1C&q9S5$z1i=it@UDp z*2|$jQB-27#VLCF`aR{C04<6dV{J!AFj424s{ZCO`8UMJ-g$jwkwPgXhFn>a=JYi( zrr|w!X_AJUy_5EXGjCgc!$`H_RIyW@=;CTS&1ng%3GmrOn-PWoE7^TkT6c@7BEgcirI*QaRQ>BDbVxwv)uRHG!b{s~Dl4N4J0Aib#^6Eo%5m3LB!hP4BJrm1zs#N4P8`##*6O;t1f ztwUB#&mGTN+G~Rtxsh^q;E&*V0O`Tk7TEEIU9Z{ z(F+`qpy1n1uApDDD&F-_wd3C@pGjA0K+R`+8po-tqdPZ{hT?)cojO4Jo~75ThzGPyuL*1( z?V7S4LhFy^Oo^l1_G&UFNOr-Bi*^+6CK8m9Om9!UZ(BcRn+gTDr^$gBWNQT6Lj=}I zb2LGb@%08VEJdY#oL2K6MbHie>_IdsdDC3V0g$&O*cK)OmEKyZTiy3W?OzJAW{k-P zF;lBkeZ)*wlSrCMtrwCbI2;z?-)=b|GHxs-04Hy=+n7KwS&6~nFSJ)!lIKN4)uc#e znm9>bMF_mfssfqmh|>vke=QI3M|JCIeXi}RhgfLTpZGCAk7VW*Lj`n%aO3!4sbs^z zV7boXX=(w_yMDa&30}G$10v833E!+hPI2SJd8_f++tQ>psw8TS?8c6r<^gxFbyI7g zl&yrg6fg_w@00}I>Hpj;OV)>tf|{W$9T@AQHFkh%p={{a@Qx_S$D*`UF=~oRklHR* z%YnOm>UMu|T4GG6Q(=%r+A~|C-Lsa2MZMC?|}2u=F(u69Oa`nd1WC?%-^D&%wd=Ja_yt@z*DuCw}bU z^^k_35iizpYe|$imzyC5<2!J{6{l2{w9s*iMxPKgIrOF-V`p6MdzPZQ*0o1Vw>Cop z^eyqt)Rj3Pe)%!S(z=@iC zBewGCcWyW^5kBly3)O#UNbUyaQu$DU{+L)*k*jTz@tRjTyc3b|DZ{vy7HBq#SzNie3>q+qL#tYub& z2=X?R5gm){u6p#t(o?6dhcV?I_bKKfo?~I@$8EEOS4ES0e`#468~`u?b0x-wD_vYY z?aW>Nlz#IyK+gxo(R~Szv0Cc9YCs!V1G|*Xfmzh)_};w_2G-sfbSu_Wo<8KZuNn^l z28z-h#CyM1Ebt&H#vw#k^U~=)X;DUXth& zj0AZfPMgx)rk)v5gQZ8X%&i}e)galC*^Z3XYGGAAf{1{x5znjFFfL+208P;9_qd}r z*x=r3$Re)QIyDO)8dv}JSdiIwetUl>NaF;pV&HP7w`$$V z>LRx$KGKcZ;H+_%_|LU0f)RD*($-e>zy!t@#8J&qUXtzfyNi}C9U?r<-*%0c%9-dI z-BM4VIZH+feRwM~JN2sf(~YrXh*eu(@$p$4Yu57H{r<%vy*f-39gSnuJ~F4Qd4pHk zdIfsOPugoh{gIKbhWW?Im?L)bnGfw{YYB&1(rv2kEw*se_aUFOOfW+*c0yyGmnZU{ zFf&bz@7WHz`)op`CU)4D*mfUjAXmNDJ%PI#_v9NyU^Q6WonT@a>mu%#U!H#wUSKr| zW!QUDN3)6CB7i9lXNBtz3x0woN6}_L{mA5ob0e;ymMT>G3stZMx$PwAY8(3}BsjFWxA| zn>SptOMi)Pax80YJ4Zv_GJseY1ebB*i$^K67Cc+i!;x`lr2?$k&RRaom@Y+K_Ke3Y zx5Pf`M)V}PBaspP`3Mjesp&TXS~NY=$SysBYv4zlxl#VbNp-)ifQFB(YzeYr)~YPC zlIPB^%ap?6t(m<;ZzWXd6Jyp%dj|h=e`NogEXVvA85b*abJri`kS9!`|FIn6E4U}& z2a7O1?9C?nh4UTvM~w}R?>P^(^q%Z-(7cG-n7X)DJZi-=67mOANz>`FIzNjmo#1R6S3 zR0hb#bcO*QG6-{AlO9vOA&%0(5?7j)H?iv3_W6Z_L(OgkX2eP*1PQdSu}d^V*HK)K zD3G+1n2bZ(ywKX$sR*DugN?SD3m%!tkx&loa#i!=hr&khsyjdE9!NL6!?^cl{tFHd z9=W_ez|!O;#;h&2!xAG^PLvozPTqTh$>* ze15w|NNoDZF#1}Z#Cn;C8HR}1OR8KpCMiGJ=811uLKh3;XMI~OufU213uqPW`ZodP z{?o1vR&A1R<-o2-JC#hSRnpN*%3q~si6Te9`Ic$VU_U~CJzh`eY% zvd-qj{LW?s6^S!1OC&BrO|sD3A<5)arOqBDHS=&$9O=*o7-y(=UfM11Xcx@{4mcV#G4-9E&V&AaJM%0G_|8a2T`os-Z{Bj6t-XA#O& zf{%&RI29nG$Sx8}usEQ?Q61iR2^iJC(`Ve3HNQ=)o;I50o6q z5=@%LflVAGF~?_io)4xNwNbo2&XuFqJE=uKO(hQ#;ePq1{bqTL$CXL-ovkqN|Yl2JK>@5$zkpp*q+ zHKlTn{pQ?)@nB31wRW6ki!0IeWQQjK;xLPUvX*Fwg467QVObJ=N z-@UwLtm01;JNuaj!Px}%nvI63y9f3s~dDusf~D6k;~HppL8I3I*IB*F%)eUCZC zgc|4$3En=6aR_Ok+s^$aGD7hp5vd$ay+p3W%}=gKXvjd*X7ZasYPSo2xO|$cNA@!c zk8gQ4?<%aU@eqjdwP;XCYt?MZ?;yyG*2Q>7c}AK(Z%68MQ*+)Y zsT`HiLEjZ~ICYvCzX2IeP9IJ_z0WN%dVKxr>UF{tWY&c_I1AQJId7$aBpvR%n%{Rd z()4sRcQO3a!tG5oRA}KK3Ry=ylk6;kj?#yB))?+P6#*Bp!n?+KS(I*1+TA~!tSZsK zVU5AqDaeK{2r@8G%qxfV7|d<>wSKyb@@lePGXH!TrTRBXU68YA(Cgo zhqpX8oeow`wnj>LOz$ep9S#MzmM;QPUkM2SctWiD^eAat%|x~HE!XT-@m4v6oKe<~ zMO@O)5=dLbx&xA;vnVw3KwlR-zHuMvcxfD5I|>`gn2|Z@jlo+z5%NU*nb;j3U+uS@ z7d~bVrmD^kjz4lb{C|?uW3{E?gJE&L1Iy{ymk}n8j(-YrhK7>u9Bk_+Jfw(T1E=RD z;+iGVO$@ZK-H;u~tk@DDNTSPrlA71(YaUjuEXMZy!-u+#x!!2nwOw$pez;W~^N4GF zIePkTzEf?WJwKLC6^o>Lspilv#u2#KiLM^BvPe;JT3`wtS{UN^&jI?B0>W5!o2+WoXH_p6daHLDrG2t+~(;=;JW~={jV!oxYmx@;Grf;w@ zSUa&Mu@M|Y?jt?=z8;BrKLxML3geKMM5xnXV^gcR;?+yV_E+iz5*vGg9ta;X?DG|+ zZtCkpv${IadqE`Q(dK^mw+Rc4OCn_i@7)>Q95B5Ro8!H^iD(PB-4u2Yt7D9lZ`gSf z4Of70i%Pb60f#@7Wg23WPwf-uA*%ez(l|GInQh87eLX$ZN2OQH&8>Tns(*lA;x*3$ z(JZ6(`e{D9<9ccV)|ooSp^8Y=f_e-E64Y5vx9j$WMnm1JS;E$$*C!G@g-+1~GnBSm z4<9=}LpePKeir-r*5)N6NXM<>W*)<$NrMCc;9QQTse`lmU*Qby{QaJosN|5&fgQRY zw?#pUS-(oM%Y1`Us=GrzDV1k{y2AQnkS%>wt+#F^zuD2U?Mlc?y0GM)s1(nI&5-}SkS=h)NR)lo=m*hMi939a6KCF|}dvYQ(rJ|zh@n8?zFBXgwgL7U-9gr;Q%nKKiI0cy!3wM6!7fGE_d@>=~Lj*GyTgQ3c~nR~cQmUgHU> z9ctNeFt{r4`7@N(z7b@M$Ch@)Al*z}B#lL}kMm~W0?IOmqB}DtwoZ<;INU{~e!NbQ zqS*0-yJBIZv_h_OO{YuWR7-2l#-_N&56iFq49W*0E4$64uF6pu(;R#lS%jX$ODK-} z1pX;@Rew8;yO~ZwqqbJ$Vn9D@%_K@H3FtZ>pGXet%5!yE zu1Hq261jKW;>;pZhB9X1wFO9-{&|5;`Daf3UrTT_{oo1V)Qf!M7)?Rq4*|uQNqY65 zVb?4|wLSCZOk&#Mqg>ykQAwbM>I3#zN}rtuGR=eouKL$t9pyX~KSAgExLZ*o?;&2N zuA3$IKR-PsN~It;a{u}XQcY$9e7X);GlVBQ0jes%!IJ}j-;76sX~T;THsD0!{O(`f z6~v2kl)oK+T(Ews%&Vs;7cAHs1waGD|6=`Ut-yf4IZwH4+^@$BmyL6Talaorz(D7j z|F@R?ewHX#W?6Cp`qu%(e>Typ#}Ah$x^WqI`7q+o6aKZqb$P-I$loXY-v#7qv+J^* zOt5n=nE(E5>+7G){TcA99iYp6HiG;Ii6+;m+08AJ)cIt##Q>8vIMRzm(TM8F?}LUvvIt>L$S@>Q4m#!=5XEUyMbB Si3R`w7xsmR-3bt%kNyW`*+Sv~ literal 0 HcmV?d00001