From d896bb9e3069a2031d00358091545a96362dc6dd Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sat, 11 Aug 2018 09:22:05 +0000 Subject: [PATCH] disable dtd processing git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1837850 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/ooxml/util/DocumentHelper.java | 9 ++-- .../poi/ooxml/util/POIXMLConstants.java | 25 +++++++++++ .../org/apache/poi/ooxml/util/SAXHelper.java | 17 ++++++- .../poi/ooxml/util/TestDocumentHelper.java | 44 +++++++++++++++++++ .../apache/poi/ooxml/util/TestSAXHelper.java | 13 +++--- 5 files changed, 96 insertions(+), 12 deletions(-) create mode 100644 src/ooxml/java/org/apache/poi/ooxml/util/POIXMLConstants.java create mode 100644 src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java diff --git a/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java b/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java index d79237d8ac..835f76e351 100644 --- a/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java +++ b/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java @@ -95,11 +95,14 @@ public final class DocumentHelper { } } - private static final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); + static final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); static { documentBuilderFactory.setNamespaceAware(true); documentBuilderFactory.setValidating(false); + trySetSAXFeature(documentBuilderFactory, XMLConstants.FEATURE_SECURE_PROCESSING, true); + trySetSAXFeature(documentBuilderFactory, POIXMLConstants.FEATURE_LOAD_DTD_GRAMMAR, false); + trySetSAXFeature(documentBuilderFactory, POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD, false); trySetXercesSecurityManager(documentBuilderFactory); } @@ -123,7 +126,7 @@ public final class DocumentHelper { Object mgr = Class.forName(securityManagerClassName).newInstance(); Method setLimit = mgr.getClass().getMethod("setEntityExpansionLimit", Integer.TYPE); setLimit.invoke(mgr, 4096); - dbf.setAttribute("http://apache.org/xml/properties/security-manager", mgr); + dbf.setAttribute(POIXMLConstants.PROPERTY_SECURITY_MANAGER, mgr); // Stop once one can be setup without error return; } catch (ClassNotFoundException e) { @@ -134,7 +137,7 @@ public final class DocumentHelper { } // separate old version of Xerces not found => use the builtin way of setting the property - dbf.setAttribute("http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit", 4096); + dbf.setAttribute(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT, 4096); } /** diff --git a/src/ooxml/java/org/apache/poi/ooxml/util/POIXMLConstants.java b/src/ooxml/java/org/apache/poi/ooxml/util/POIXMLConstants.java new file mode 100644 index 0000000000..ab58e35833 --- /dev/null +++ b/src/ooxml/java/org/apache/poi/ooxml/util/POIXMLConstants.java @@ -0,0 +1,25 @@ +/* ==================================================================== + 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.ooxml.util; + +public class POIXMLConstants { + public static final String FEATURE_LOAD_DTD_GRAMMAR = "http://apache.org/xml/features/nonvalidating/load-dtd-grammar"; + public static final String FEATURE_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd"; + public static final String PROPERTY_ENTITY_EXPANSION_LIMIT = "http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit"; + public static final String PROPERTY_SECURITY_MANAGER = "http://apache.org/xml/properties/security-manager"; +} diff --git a/src/ooxml/java/org/apache/poi/ooxml/util/SAXHelper.java b/src/ooxml/java/org/apache/poi/ooxml/util/SAXHelper.java index 806aede583..f3b1b7f6a5 100644 --- a/src/ooxml/java/org/apache/poi/ooxml/util/SAXHelper.java +++ b/src/ooxml/java/org/apache/poi/ooxml/util/SAXHelper.java @@ -68,6 +68,9 @@ public final class SAXHelper { saxFactory = SAXParserFactory.newInstance(); saxFactory.setValidating(false); saxFactory.setNamespaceAware(true); + trySetSAXFeature(saxFactory, XMLConstants.FEATURE_SECURE_PROCESSING, true); + trySetSAXFeature(saxFactory, POIXMLConstants.FEATURE_LOAD_DTD_GRAMMAR, false); + trySetSAXFeature(saxFactory, POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD, false); } catch (RuntimeException | Error re) { // NOSONAR // this also catches NoClassDefFoundError, which may be due to a local class path issue // This may occur if the code is run inside a web container @@ -81,6 +84,16 @@ public final class SAXHelper { } } + private static void trySetSAXFeature(SAXParserFactory spf, String feature, boolean flag) { + try { + spf.setFeature(feature, flag); + } catch (Exception e) { + logger.log(POILogger.WARN, "SAX Feature unsupported", feature, e); + } catch (AbstractMethodError ame) { + logger.log(POILogger.WARN, "Cannot set SAX feature because outdated XML parser in classpath", feature, ame); + } + } + private static void trySetSAXFeature(XMLReader xmlReader, String feature) { try { xmlReader.setFeature(feature, true); @@ -101,7 +114,7 @@ public final class SAXHelper { 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); + xmlReader.setProperty(POIXMLConstants.PROPERTY_SECURITY_MANAGER, mgr); // Stop once one can be setup without error return; } catch (ClassNotFoundException e) { @@ -117,7 +130,7 @@ public final class SAXHelper { // separate old version of Xerces not found => use the builtin way of setting the property try { - xmlReader.setProperty("http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit", 4096); + xmlReader.setProperty(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT, 4096); } catch (SAXException e) { // NOSONAR - also catch things like NoClassDefError here // throttle the log somewhat as it can spam the log otherwise if(System.currentTimeMillis() > lastLog + TimeUnit.MINUTES.toMillis(5)) { diff --git a/src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java new file mode 100644 index 0000000000..c8147c2800 --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java @@ -0,0 +1,44 @@ +/* ==================================================================== + 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.ooxml.util; + +import org.junit.Test; +import org.xml.sax.InputSource; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; +import java.io.ByteArrayInputStream; + +import static org.junit.Assert.*; + +public class TestDocumentHelper { + @Test + public void testDocumentBuilder() throws Exception { + DocumentBuilder documentBuilder = DocumentHelper.newDocumentBuilder(); + assertNotSame(documentBuilder, DocumentHelper.newDocumentBuilder()); + assertTrue(documentBuilder.isNamespaceAware()); + assertFalse(documentBuilder.isValidating()); + documentBuilder.parse(new InputSource(new ByteArrayInputStream("".getBytes("UTF-8")))); + } + + @Test + public void testDocumentBuilderFactory() throws Exception { + assertTrue(DocumentHelper.documentBuilderFactory.getFeature(XMLConstants.FEATURE_SECURE_PROCESSING)); + assertFalse(DocumentHelper.documentBuilderFactory.getFeature(POIXMLConstants.FEATURE_LOAD_DTD_GRAMMAR)); + assertFalse(DocumentHelper.documentBuilderFactory.getFeature(POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD)); + } +} diff --git a/src/ooxml/testcases/org/apache/poi/ooxml/util/TestSAXHelper.java b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestSAXHelper.java index 825cdf40f5..4a2fa80af4 100644 --- a/src/ooxml/testcases/org/apache/poi/ooxml/util/TestSAXHelper.java +++ b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestSAXHelper.java @@ -16,10 +16,7 @@ ==================================================================== */ package org.apache.poi.ooxml.util; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import java.io.ByteArrayInputStream; @@ -35,10 +32,12 @@ public class TestSAXHelper { XMLReader reader = SAXHelper.newXMLReader(); assertNotSame(reader, SAXHelper.newXMLReader()); assertTrue(reader.getFeature(XMLConstants.FEATURE_SECURE_PROCESSING)); + assertFalse(reader.getFeature(POIXMLConstants.FEATURE_LOAD_DTD_GRAMMAR)); + assertFalse(reader.getFeature(POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD)); assertEquals(SAXHelper.IGNORING_ENTITY_RESOLVER, reader.getEntityResolver()); - assertNotNull(reader.getProperty("http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit")); - assertEquals("4096", reader.getProperty("http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit")); - assertNotNull(reader.getProperty("http://apache.org/xml/properties/security-manager")); + assertNotNull(reader.getProperty(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT)); + assertEquals("4096", reader.getProperty(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT)); + assertNotNull(reader.getProperty(POIXMLConstants.PROPERTY_SECURITY_MANAGER)); reader.parse(new InputSource(new ByteArrayInputStream("".getBytes("UTF-8")))); }