From b50ce609ca5f41b6958b5c2ab2ab104f83870a99 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 1 Aug 2025 20:28:05 +0100 Subject: [PATCH] check xwpf node depth (#869) * check xwpf node depth * Update TestAllFiles.java * Update TestAllFiles.java --- .../org/apache/poi/stress/TestAllFiles.java | 3 + .../org/apache/poi/ooxml/POIXMLException.java | 2 +- .../poi/xwpf/usermodel/XWPFDocument.java | 26 ++++---- .../org/apache/poi/xwpf/TestXWPFBugs.java | 10 +++ .../java/org/apache/poi/POIException.java | 59 ++++++++++++++++++ .../java/org/apache/poi/util/XMLHelper.java | 36 ++++++++++- test-data/document/deep-table-cell.docx | Bin 0 -> 17198 bytes 7 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 poi/src/main/java/org/apache/poi/POIException.java create mode 100644 test-data/document/deep-table-cell.docx diff --git a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java index b3aff917ac..957b804ff5 100644 --- a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java +++ b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java @@ -97,6 +97,9 @@ public class TestAllFiles { "poifs/60320-protected.xlsx", "poifs/protected_sha512.xlsx", + // stress docs + "document/deep-table-cell.docx", + // NOTE: Expected failures should usually be added in file "stress.xls" instead // of being listed here in order to also verify the expected exception details! }; diff --git a/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java b/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java index f949eccb90..be53202057 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java +++ b/poi-ooxml/src/main/java/org/apache/poi/ooxml/POIXMLException.java @@ -19,7 +19,7 @@ package org.apache.poi.ooxml; /** * Indicates a generic OOXML error. */ -public final class POIXMLException extends RuntimeException{ +public final class POIXMLException extends RuntimeException { /** * Create a new {@code POIXMLException} with no * detail message. diff --git a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java index eb8c4c30c9..56db1b0420 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java @@ -23,22 +23,11 @@ import static org.apache.poi.ooxml.POIXMLTypeLoader.DEFAULT_XML_OPTIONS; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Deque; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Optional; -import java.util.Spliterator; +import java.util.*; import javax.xml.namespace.QName; import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; import org.apache.logging.log4j.Logger; +import org.apache.poi.POIException; import org.apache.poi.logging.PoiLogManager; import org.apache.poi.common.usermodel.PictureType; import org.apache.poi.ooxml.POIXMLDocument; @@ -61,6 +50,7 @@ import org.apache.poi.poifs.crypt.HashAlgorithm; import org.apache.poi.util.IOUtils; import org.apache.poi.util.Internal; import org.apache.poi.util.Removal; +import org.apache.poi.util.XMLHelper; import org.apache.poi.wp.usermodel.HeaderFooterType; import org.apache.poi.xddf.usermodel.chart.XDDFChart; import org.apache.poi.xwpf.model.XWPFHeaderFooterPolicy; @@ -105,6 +95,7 @@ import org.openxmlformats.schemas.wordprocessingml.x2006.main.StylesDocument; @SuppressWarnings("unused") public class XWPFDocument extends POIXMLDocument implements Document, IBody { private static final Logger LOG = PoiLogManager.getLogger(XWPFDocument.class); + private static final int MAX_NODE_DEPTH = 1000; protected List footers = new ArrayList<>(); protected List headers = new ArrayList<>(); @@ -214,6 +205,13 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody { doc = DocumentDocument.Factory.parse(stream, DEFAULT_XML_OPTIONS); ctDocument = doc.getDocument(); } + final int nodeDepth = XMLHelper.getDepthOfChildNodes(ctDocument.getDomNode(), MAX_NODE_DEPTH); + if (nodeDepth > MAX_NODE_DEPTH) { + throw new IOException(String.format(Locale.ROOT, + "The document is too complex, it has a node depth of %s, which exceeds the maximum allowed of %s", + nodeDepth, + MAX_NODE_DEPTH)); + } initFootnotes(); @@ -304,6 +302,8 @@ public class XWPFDocument extends POIXMLDocument implements Document, IBody { } } initHyperlinks(); + } catch (POIException e) { + throw new IOException(e); } catch (XmlException e) { throw new POIXMLException(e); } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java b/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java index c39c7fe7cd..170473396e 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xwpf/TestXWPFBugs.java @@ -28,6 +28,7 @@ import javax.crypto.Cipher; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipFile; import org.apache.poi.POIDataSamples; +import org.apache.poi.POIException; import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.openxml4j.opc.PackagePart; import org.apache.poi.openxml4j.opc.PackagePartName; @@ -274,4 +275,13 @@ class TestXWPFBugs { assertEquals(STJcTable.END, tbl5.getCTTbl().getTblPr().getJc().xgetVal().getEnumValue()); } } + + @Test + public void testDeepTableCell() throws Exception { + // Document contains a table with nested cells. + IOException ex = assertThrows(IOException.class, + () -> XWPFTestDataSamples.openSampleDocument("deep-table-cell.docx")); + assertInstanceOf(POIException.class, ex.getCause()); + assertTrue(ex.getMessage().contains("Node depth exceeds maximum supported depth")); + } } diff --git a/poi/src/main/java/org/apache/poi/POIException.java b/poi/src/main/java/org/apache/poi/POIException.java new file mode 100644 index 0000000000..106f82596a --- /dev/null +++ b/poi/src/main/java/org/apache/poi/POIException.java @@ -0,0 +1,59 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi; + +/** + * Indicates a generic POI exception. This is not commonly used in POI + * but this is intended to be a base class for some new POI exceptions. + * Historically, POI has used {@link RuntimeException} for most of its + * exceptions, but this is not a good practice. This class is a checked + * class that extends {@link Exception} so needs to be explictly + * caught or declared in the method signature. + * + * @since POI 5.5.0 + */ +public class POIException extends Exception { + private static final long serialVersionUID = 1L; + + /** + * Create a new {@code POIException} with the specified message. + * + * @param msg The error message for the exception. + */ + public POIException(String msg) { + super(msg); + } + + /** + * Create a new {@code POIException} with the specified cause. + * + * @param cause the cause of this exception + */ + public POIException(Throwable cause) { + super(cause); + } + + /** + * Create a new {@code POIException} with the specified message and cause. + * + * @param msg The error message for the exception. + * @param cause the cause of this exception + */ + public POIException(String msg, Throwable cause) { + super(msg, cause); + } +} diff --git a/poi/src/main/java/org/apache/poi/util/XMLHelper.java b/poi/src/main/java/org/apache/poi/util/XMLHelper.java index efeca688a5..071ae08f54 100644 --- a/poi/src/main/java/org/apache/poi/util/XMLHelper.java +++ b/poi/src/main/java/org/apache/poi/util/XMLHelper.java @@ -30,6 +30,7 @@ import static javax.xml.stream.XMLOutputFactory.IS_REPAIRING_NAMESPACES; import java.io.StringReader; import java.lang.reflect.Method; +import java.util.Locale; import java.util.concurrent.TimeUnit; import javax.xml.parsers.DocumentBuilder; @@ -49,7 +50,9 @@ import javax.xml.validation.SchemaFactory; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogBuilder; import org.apache.logging.log4j.Logger; +import org.apache.poi.POIException; import org.apache.poi.logging.PoiLogManager; +import org.w3c.dom.Node; import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -77,7 +80,6 @@ public final class XMLHelper { "org.apache.xerces.util.SecurityManager" }; - private static final Logger LOG = PoiLogManager.getLogger(XMLHelper.class); private static long lastLog; @@ -253,6 +255,38 @@ public final class XMLHelper { return factory; } + /** + * Counts the depth of the DOM tree starting from the given node. + * + * @param node the node to check + * @param maxSupportedDepth the maximum supported depth of the DOM tree + * @return the depth + * @throws POIException if the depth exceeds maxSupportedDepth + */ + public static int getDepthOfChildNodes(final Node node, final int maxSupportedDepth) throws POIException { + return getDepthOfChildNodes(node, maxSupportedDepth, 0); + } + + private static int getDepthOfChildNodes(final Node node, final int maxSupportedDepth, + final int nodeDepth) throws POIException { + final int currentDepth = nodeDepth + 1; + int maxDepth = currentDepth; + Node child = node.getFirstChild(); + while (child != null) { + int childDepth = getDepthOfChildNodes(child, maxSupportedDepth, currentDepth); + if (childDepth > maxDepth) { + maxDepth = childDepth; + if (maxDepth > maxSupportedDepth) { + throw new POIException(String.format(Locale.ROOT, + "Node depth exceeds maximum supported depth of %s" , + maxSupportedDepth)); + } + } + child = child.getNextSibling(); + } + return maxDepth; + } + private static Object _xercesSecurityManager; private static volatile boolean _xercesSecurityManagerSet = false; diff --git a/test-data/document/deep-table-cell.docx b/test-data/document/deep-table-cell.docx new file mode 100644 index 0000000000000000000000000000000000000000..6bb54e6c6d91f95bfd39049fa3121b777255e657 GIT binary patch literal 17198 zcmeI4dr(u^9mk_6Y^y9?E28oUT4-y9i&|G4SDHn1i;WLf5mQV8MsOD&pwR@E00B|a zvb&(J#`=KzL?M!Z&m@EtSXSAH=qlVGhVU?89z0Y+Ovugcfjgb<+LcL}@vk2EP449P zJD+=Uzq!NwzH<*Jg6cGGGKoZbkJK$`j`lb~LJiNQx3uok!oY4#N5WZ`$c0am zu4z~|+?d^$#njU5Q#YNKJ@hlQtE%r2kGc#pJx^Z)W{+-i1lx?`fZ$dAMzdUs^LK`N;*^tj0_CeS9~sp5NNI-c&BYbVrut zhlb>z*hOHCs;ITX66?rs=A>m@xJ9*=y-srT;Y6@y4vA`tXlE>dDVHX_Y|{KhcJA)ov95 zO#*;66TmTs9lvc?5Hj@oRsBcrSIwD7dT?g#(w2m1=Y0`j9{V1jWP+#jBw$te1L=GnE(-23m{-d$*Vf2zOk`swnpF@KyiKYLx; zcgrs=4y&GeF#g-a88a@5yG}pOh+5O;B&F-puFuoXneKBz`j@QKwZ}L3P+V#M5H^fu z70-@Ud-ai@nz74$&l~E_Q7PPjMP&i!EB#vA&UaeRAoC_HE~r-IFD@tj^twK+{C?Wv zk4YpgdlKmba9zns?6{z~q)nXd;ENyn#=D=+t?#Mb+1Zs!8DIHSsVma!w}d`(b~fId z)B2&;+GoNjQtgcJlIzw1dk(eoeXm8*2epg4Y@IrD$T9UplNr}3aC!4zzuz}}Y&a=4`c7vKr$_G=Vei}GK0i}>b8fiieMLcEHU(EJY%jZTQxMT=t$18AkPY>0*6s)a_kNq9*dz6Hm z)@!}|<>Y|v~`;JTzX{IybH@3~WeDK*$D@DY^-cvlQw-J5QU!TVP$FiTLn$*g#7 z4zhRGDolJ{f3Btrw>|2u2-Z*#eKy%%5QLj%^D(Q2iFGEC`GXvVMPZK-RZ#5O82q(2 z-i+G~eEb82rCBuCp-98{<(39~prZhP*^$H_kdblA9Svs8R-|`}V(gD%@WDSn6JPY1 zqThJMCrZC@Lx`}U`s)LA_N__GV+Dr~r^}jLM~EMc&I#PC`LfWHYTvrVyv@Pk*E}yYxe{qFZq^7f zQ5JeY?OUCgw=wvx*fUP$I?w1ljhjWJ1!~_B!r;Q)CnT%f%hF>%yKBu<&j5*oAmI-Z zg+qy%An~P;{Pe?_N3Zlu&wLn3Te-g}Qnhk_TO@7OeqAKGih<-y=^-3Rs-BHK){ZSw zxe=26+9gHGW1w&aLDw*lOH%quj^wnS9nzrP3y5_BY7v4) zGLRxEeH}+4)Uz24+A2VRLNS6yF_01|oxzcm>)B}y+Ofq-_b35Xh@dnEQYNLxb0qil z>@#4FV&ySVxQd|B4CJPizMUg^s%Oi=9DrCapq3ygoqHrZ3QyrNn4=}Yx@B}aM}aF3_ns+4^j|5rtGE2F(i zsM+3!>`Z9vU9d-Rd!ZrOvz$uLH_2RE{E-J%wR7(z!RJ_FR`Q{o>*>Q-^f0#M>M(YN zz-RjB-T9V~EYt z*j`R$i(#)E9nTZN!ibVg7n@-?sK80F8HR%jM2pQZ98^HMv6oZbVmKg2U3ntU0cNmK zo~Yuf!5=bM7*P@+7Yqj#a2a5_8Rb4IUWnM765DI~XNw_Ijymx~urQ(|i^OIa4l0nc zvDZ}9V%RT7C-6ib1I!?!oT=i;!3_rsZ_PI7J)i=6 zzyS83XUHB1hV6lg=!H{=URX}_!YxEE+&kPtmzalJXp}eHLJyr8xQhQ{4BCTH8>8va z9yqMPNoWroRv;SM1BVrWE?|_#$idDS4g(567cim=c&lS_urr3kfCA73jK&xNdtia~ z;2nAxXb+%0nC@_wKkAVu9d3@{$Y_U~V>lAp185K4`C|}h51>6*^p2l|KzlGsd!Pve zj|SYD7%xNOl55R*;0eOdC)yb8@TmbcO+IH&+zMKqd)RT$pS#H|DTRGUtN&lOcz z^~LZ7YVhOx0N;dV7o-o06jnEU0Q^^d|0#T6kc{^UVlYhvb3o4YU&IEj=o#tWR{M|(273BI|_!GFMh0RGpe2NBfq&O<|-z^?=xW0nlP F`xnPTc@F>p literal 0 HcmV?d00001