From f8a990051bc3be17161d7916c496c941cb231eae Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 1 Jul 2024 21:36:31 +0000 Subject: [PATCH] throw exception if xlsx contains duplicate file names git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1918800 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/openxml4j/opc/ZipPackage.java | 2 ++ .../opc/internal/InvalidZipException.java | 14 ++++++++++++++ .../poi/openxml4j/util/ZipSecureFile.java | 18 ++++++++++++++++++ .../poi/xssf/usermodel/TestXSSFWorkbook.java | 13 +++++++++++++ test-data/spreadsheet/duplicate-file.xlsx | Bin 0 -> 10721 bytes 5 files changed, 47 insertions(+) create mode 100644 poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/InvalidZipException.java create mode 100644 test-data/spreadsheet/duplicate-file.xlsx diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java index 46b3fd3b7f..7811627c33 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java +++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java @@ -187,6 +187,8 @@ public final class ZipPackage extends OPCPackage { try { final ZipFile zipFile = ZipHelper.openZipFile(file); // NOSONAR ze = new ZipFileZipEntrySource(zipFile); + } catch (InvalidZipException e) { + throw new InvalidOperationException("Can't open the specified file: '" + file + "'", e); } catch (IOException e) { // probably not happening with write access - not sure how to handle the default read-write access ... if (access == PackageAccess.WRITE) { diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/InvalidZipException.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/InvalidZipException.java new file mode 100644 index 0000000000..9a788b2134 --- /dev/null +++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/InvalidZipException.java @@ -0,0 +1,14 @@ +package org.apache.poi.openxml4j.opc.internal; + +import java.io.IOException; + +/** + * Thrown if the zip file is invalid. + * + * @since 5.3.1 + */ +public class InvalidZipException extends IOException { + public InvalidZipException(String message) { + super(message); + } +} diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java index 60dcb1d53c..ff183a696e 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java +++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java @@ -19,11 +19,15 @@ package org.apache.poi.openxml4j.util; import java.io.File; import java.io.IOException; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Set; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipFile; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.poi.openxml4j.opc.internal.InvalidZipException; import org.apache.poi.util.Internal; import org.apache.poi.util.Removal; @@ -205,6 +209,7 @@ public class ZipSecureFile extends ZipFile { public ZipSecureFile(File file) throws IOException { super(file); this.fileName = file.getAbsolutePath(); + validateEntryNames(); } /** @@ -214,6 +219,7 @@ public class ZipSecureFile extends ZipFile { public ZipSecureFile(String name) throws IOException { super(name); this.fileName = new File(name).getAbsolutePath(); + validateEntryNames(); } /** @@ -246,4 +252,16 @@ public class ZipSecureFile extends ZipFile { public String getName() { return fileName; } + + private void validateEntryNames() throws IOException { + Enumeration en = getEntries(); + Set filenames = new HashSet<>(); + while (en.hasMoreElements()) { + String name = en.nextElement().getName(); + if (filenames.contains(name)) { + throw new InvalidZipException("Input file contains more than 1 entry with the name " + name); + } + filenames.add(name); + } + } } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java index 744887ff40..bd36492535 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java @@ -27,6 +27,7 @@ import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.ooxml.POIXMLProperties; import org.apache.poi.ooxml.TrackingInputStream; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; +import org.apache.poi.openxml4j.exceptions.InvalidOperationException; import org.apache.poi.openxml4j.opc.ContentTypes; import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.openxml4j.opc.PackageAccess; @@ -1447,6 +1448,18 @@ public final class TestXSSFWorkbook extends BaseTestXWorkbook { } } + @Test + void testDuplicateFileReadAsFile() { + assertThrows(InvalidOperationException.class, () -> { + try ( + OPCPackage pkg = OPCPackage.open(getSampleFile("duplicate-file.xlsx"), PackageAccess.READ); + XSSFWorkbook wb = new XSSFWorkbook(pkg) + ) { + // expect exception here + } + }); + } + @Test void testWorkbookCloseClosesInputStream() throws Exception { try (TrackingInputStream stream = new TrackingInputStream( diff --git a/test-data/spreadsheet/duplicate-file.xlsx b/test-data/spreadsheet/duplicate-file.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..5853cf1ded3e1fdc172863b15ab45d8b92f481c5 GIT binary patch literal 10721 zcmeHMbyQW|wm&q8G}6)`ARW>v-AH#g9J;$3q@|G(kdl&a>244Y9nvUW@8Eauh0piy z9q<3^++**(_84o;vDaL)fAUgL(3k*tcz6H+KmxuW2m=5QsVNM!aWuAZ)Kzk`HFnUZ zceS=6eh&@#A{zktF!9gmzfl6EN<*@cOsFks=g4U~eKzd@Ou<0`{9+VnMA86oD-}p8u>){jG51BBM7pVWQt1 zE?*5Q{AlaafsQ@;gDT}+UX0CGvFxw#bFQ0MGE+?dPlYWV7=a-vhQIc^|K<;6I{Kz zLnG=c$7f%?z(OBF6U?iDY5GYfmDiJi|4gt|Va;c01=idCz_a&fR%8o6UJ4FgS`ZRV z4gvs(g$97X|D9bTg6&e*-q^~4f&Tva#~%M1t?z&DxioIbqLcBd??FIwz*w`wd^=9A zEQ8_b0?r(iS?#ef`oOGBa?4Fq=Pac5w!YZFc%huo!Xi)9_g<3I71oat=$PDOwJxQ> zb(d}@02)gBBvHGM%Ux*BQ+q?FiDHuVFPxiWXx?Z|j^?isq2+G7zE=x1P4>Y|r0)ltm3iv7!}yfWF8T~}KOQRQq`m$SEh5FN zLIt*9KjWQ$I4p<7(;6U{6Z3P89!_|tx1!!++00dgz$FEW)1z;K@s&`iP zov&-*EHtE`OfjlIR#8zgt7TnbKMYkRBn~cBPVptkg_5M&l1Tm)qSY{;&zV}xO12e)mo6O8S+O!mX7;(qx$S0;6-&J!DK=deDR#_Y%3qWkL zuos!}a+(P)&`h)+QD>AuMMcZ#wLkT=i8{PwoV{?-pyGtk2cSvd`XyRBwl?FrMYjRo zv1{^}7NH=tOc?a#dhj#jfTV8iC1%pv#*R=o6+jn;)H}~IK_|M1n>nscf1Z5e#*Sze z1lP+$1L%yY*Z%n^Im}pyI>(7kfGvz|luIUNCT|z$sUDWCV53fw-FN-^h*Ni@DtD1n zz;?zJxx3h^C6guCOd{wf5se>MIzG^$E6d20aT1^s_$|9W+Ilt=^Vy~QZ z#yrH9%{PY0T|&IKOBZSqeonMETHD-i%?KoO=QAV;Dlyd;5n22^68;EfXk6D|G-qiXxDN(4*N)^$&v z+lF9WeIL#=e$2W>;aIn6i>- zb-P|S*0JPTeqn-~>d&92m2e?`SGv4)SEU|1CATFAlMSFYJ+e^=ZTx&8EhUi<&ayY* ziq^udB20GXb?(>?;bh&Kau|wZt=QYBo_kx!W9Rd%JAC za7y!y3JjxII?wpBsp%?0ym9iKk@R}%4Kp|bQOg1KAd>8N&C%|N^zM^`uuOxpFRk4o z*ZJ>HUfESWx*BZnV^EL1hGXTj+G7v!A5T8vS2XPb(VlRtdCNVOm?%Y)mnkkj(I2mN z_HY@5P^mzfLzZCMYdr9cvqC~qpo`r1JU#H?!;iZPC{jlpwydU(*+etdd$uv3M16O0 z%yBjN4rP4flPDs!56s9x)lP{~jLi=1ucPlTK~1ee)lt^V%QA#YB8B;N^q4Jn#6P^> z7Il+`+XTIxKi;8rw&1niqhHonFoQw);0$Yp!C) zRdj}{=@c>@x5k@>j;p??h%n?_l*ZZi=gSWah2&S9rp2PPA%qc(REc%X>B(ENUlEEZ zi#FLB;Bi6nZ#5R5pR+n23GRRI)s_xOCV~bLwjT`aOmk-xUc3C9Hkn+VGeat`yjfPS zT!=bnsWnBrv8xa(XvdD5z8x!f?lu=qtBH7BT}UAmWhHp%!@~ESGV0}Wnqz)#=9D}tPpZ$#Aky}!b6Y{3O1d>8 zG;6NvSi9d&6NM>^0ZxWZL>R{{%cs_R)zDylgFXdT4cGrzi(vfa6dcWrt&JIeJ%9Ow zJq^v61$HcNUA;S~92YTcsak?AYESFbLGpli65)K~%+^c=ul@2&zo_~Nhy4KIj=-NWHiY2X>x^FnU=W$py+h%zC*XEK#ZZG{Ok6at*s zPtqYP`$i!}(O>mKuj@s+i7tJC#FeH_UX0$9&ZxZ}YHi_C#0sfH`zV2}jmE##e*qw5 zN)Kys(zU3uh|Ti3-0uECq{DM})Lod>;(ANR(J`Qgv$o&vsGfYa4$Qi{D&hC{@w%JQ zZ8@*P9oacg23>EjgYJ%KPK0t#k`Hul4;M#ty-!EVTN&Hno(_LMbVw&gCVVetD~Z7= z*a1J() zl6|qvtB!j4lQYk%Q2wk=rN0>rx_7eMv0z5Bv21*of>|U za3sfYH5MWil-%iM(vC7nF%li-h}xNrIhPE60ZpZZBO&hBK}MyAQ6^riR9 zs(xOhPeWcGdMv)lq9q$E>dfE}MATz_GBuOHKPLd5qL089AQ;J{5|#aoeK&~1UNA03 zu3=t;S>-vFD#5dY;7L|W#WdML19Wg`vQfLNwUqAXxp~ICRcdTx;A6OuF6=)D;egD< z!JMy2{fRwdZ9pQdG|C>)Yi$3k-oX?%f~>CUHBVsVdiRgAQ#);S$1CHL)#j1;&1PlZ z8QlZc^~4*RLWDDZwS~xKE?z-6IuIu7E0`e}!5_M`X3lJp+O1Q$ z)#OoUURWuyER`f%p(Wo#25R`T`emo&g{G_vUM7XC+v3^uWplwshR0Nfju@o$>1(ld z#3dIe6Bs27jQ3egmVPgHvX|am*XD8YFn#kxpmN8>zgSnr^8MDHqf4D2?MZRXniHg- z;^hXpMZh-sJS_U+Y1&)cj2(k9mJ0Yq)yfJ>owsQiz85k_Z45Ylw(%sqBR5LDz-EuV zOdI9`KI;+2=1d>!%xUq-4jC#1#7;FE!mGuZ-5s5gU5gR%T>KUk-Gz{PzdUzDt&Q}C z`b>BeFG~`Jj5Yl@i69<1<=B@KEziQn$6G0Dj3OhL*6bkO6h46>S0jdnmo=)Gxe*x0 ztIyP8ZB@3{*$)Kaae9CT%qj)mnr+Rot=9{M9qn@a|A5t&8aF+huBW0t`R+D?0>)G=hQ99N)mT5(7)`T-^Xp_M^tg}Ow zKR38-;I0%bXP!t+akP^P7)vP(%ciXG<^GUy6L+nY4Cl0-07%G< zK`vb+O24CAO+bk$A$Qhz8Vl4(%pNOl&(hAfV>hWCPthCZWN9jxG} zGnL;?WXLxb3AwH?d`#r@4(NE9i`#(j{H}1Mb2=4Y!_5rdII-!|>YpG7?y$8&k(82} zJ2jb?FXB6EG@t6tANO=tOyN5Pm)e<5K9}psCEk_EQeP7CyAkwQbD`8PS&bH@Yh7LY z*{M^qqM#`gHm;;yH8H#UYP(#1(`(&R6zJ-6+h3o_cJwcyY*CmZpp3f1`yPA|P zc^eBc^9u>xjLajc=PsHKc_B?V#7sNU&f{k;XPph8 z{Fw4WgR8RTM5LT~iLlXU|BO7oaos|Bu#={ za@CO7-DtxcDe;xo6;EbxodhOqw-XLbJaq<4)wUBlrUYhrLHxRxBDv{&yy`8M5V}b2 zxzX}xBY#?FY9FutAji_@sjoHE6kc>3PTQUN!w|av5xYs!^ywXDg_oJd*+uAW-Y`wJVSJ zLi4H>dp|mQ-KY{9&I%urUk?`H*~0Qu(a9f6n_ZI~A<$fPAayjh^?~t@yap}Q{Hz!5 zMdLzRNv8c{mimMWNz&f2VdATiJq>*NKmFQ&nj6z;Gt!34NY1ooBrbhyD6RZ3ydtNT zx{r9jE0a|5w+Y~X{sg}N{jSi!{ShOep}alN)`7tgXm5OPnBWHcpSB2CY*BFva^R!w z0m)n=OM%2*33S7Vz7M97k?u0{EZKudEVZ8R?5jJQ{Q~F=0J@yrfy!qrdnU3zg|@WA zSq}x~HwTL7|1B9k5@|h-ROv&seZoOSDQp>lJ&f;}PB1rOa z!VnZVPkHw8ua`s)jX4(NX~l*WSEt+6cXW2Y@(a3rDK2)O%B~&1ruOl1_be*Mv#eT3 zgieY{9F#YNxJV%q3h&tXfk@82PM@RZj%wyb=7>=0hP2S{{!E;+ueX3$y!`q2Dw`J- zj6F8#5=QfeP(Z^1dtfUYjobNjj$gXb9($UdfJk-F$GJHjH~g>sgQhPJsBZMrzjjG* z$pNcM$fZJv8pC8SQQAK7-raQxCPhbwdI<2gke`oVo_dFTyv>E(j&I6tlWAMx0{~k|JAFHmut?eJp1*_~oC?i%z4h)S1ABbOvyf|aS6~{;& zmtK|9qU4BN%$?jL)!>%wn7s9PiUDJ{wlPzFbKYt7B?p^IoD+50UfrA;z{>{FsL`@d zg#Ai!ZKcAFmt@l3|K+RG(qREN$}+k2*3sbWP4A0}EYcqz=f2Fs9X!!BcWI~UpTs(1 zbl>V`IfZEd0Ae%Q9CeEQsNmozChW*zXAzx=llA#(Z`xRIHO6FR`C6k%M^t(JC!vOcq{s%&N`me+IU!c!pF87eZ@z2`|k1s zk)!_N@_VoO@o(HScwE2S7ugS_gZ+!aL&^R7(#J)@eSz~p&+-0R`g^(aJBP=)+I=zc zz~Kj&!=KXPK^}h_{VvAGIm~@x`!Koe`6Ds@Npl|r10Kms!GL`y0DuU-_Q8&Dk?8*G Fe*io98I}M5 literal 0 HcmV?d00001 -- 2.39.5