entity_expansion

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1838135 13f79535-47bb-0310-9956-ffa450edef68

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 835f76e351edc3a80a27d801831920359362a672..684564c1b6a1a0c4b74ebde3fd325237a356c88c 100644 (file)
--- a/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java
+++ b/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java
@@ -99,7 +99,9 @@ public final class DocumentHelper {
     static {
         documentBuilderFactory.setNamespaceAware(true);
         documentBuilderFactory.setValidating(false);
-
+        //this doesn't appear to work, and we still need to limit
+        //entity expansions to 1 in trySetXercesSecurityManager
+        documentBuilderFactory.setExpandEntityReferences(false);
         trySetSAXFeature(documentBuilderFactory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
         trySetSAXFeature(documentBuilderFactory, POIXMLConstants.FEATURE_LOAD_DTD_GRAMMAR, false);
         trySetSAXFeature(documentBuilderFactory, POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD, false);
@@ -125,7 +127,7 @@ public final class DocumentHelper {
             try {
                 Object mgr = Class.forName(securityManagerClassName).newInstance();
                 Method setLimit = mgr.getClass().getMethod("setEntityExpansionLimit", Integer.TYPE);
-                setLimit.invoke(mgr, 4096);
+                setLimit.invoke(mgr, 1);
                 dbf.setAttribute(POIXMLConstants.PROPERTY_SECURITY_MANAGER, mgr);
                 // Stop once one can be setup without error
                 return;
@@ -137,7 +139,8 @@ public final class DocumentHelper {
         }
 
         // separate old version of Xerces not found => use the builtin way of setting the property
-        dbf.setAttribute(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT, 4096);
+        // Note: when entity_expansion_limit==0, there is no limit!
+        dbf.setAttribute(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT, 1);
     }
 
     /**

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 38b17081591a3b40cac6879f5e0d44060f7f3e28..2ebd9f8141f2b3848ce74c9981781d5113c8bb31 100644 (file)
--- a/src/ooxml/java/org/apache/poi/ooxml/util/SAXHelper.java
+++ b/src/ooxml/java/org/apache/poi/ooxml/util/SAXHelper.java
@@ -113,7 +113,7 @@ public final class SAXHelper {
             try {
                 Object mgr = Class.forName(securityManagerClassName).newInstance();
                 Method setLimit = mgr.getClass().getMethod("setEntityExpansionLimit", Integer.TYPE);
-                setLimit.invoke(mgr, 0);
+                setLimit.invoke(mgr, 1);
                 xmlReader.setProperty(POIXMLConstants.PROPERTY_SECURITY_MANAGER, mgr);
                 // Stop once one can be setup without error
                 return;
@@ -130,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(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT, 4096);
+            xmlReader.setProperty(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT, 1);
         } 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/TestSAXHelper.java b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestSAXHelper.java
index 4a2fa80af4543922519bc19b1c7f85da0455bfd5..1190c476125a918da73a6b29d6907e12294d3c89 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/ooxml/util/TestSAXHelper.java
+++ b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestSAXHelper.java
@@ -36,7 +36,7 @@ public class TestSAXHelper {
         assertFalse(reader.getFeature(POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD));
         assertEquals(SAXHelper.IGNORING_ENTITY_RESOLVER, reader.getEntityResolver());
         assertNotNull(reader.getProperty(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT));
-        assertEquals("4096", reader.getProperty(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT));
+        assertEquals("1", reader.getProperty(POIXMLConstants.PROPERTY_ENTITY_EXPANSION_LIMIT));
         assertNotNull(reader.getProperty(POIXMLConstants.PROPERTY_SECURITY_MANAGER));
 
         reader.parse(new InputSource(new ByteArrayInputStream("<xml></xml>".getBytes("UTF-8"))));

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 8d73577838693bb471d634fa746c8a3ca93f03d5..056c15a37343e036477648e221920247769acba7 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
@@ -44,7 +44,8 @@ import java.util.Set;
 import java.util.TimeZone;
 import java.util.TreeMap;
 
-import org.apache.poi.EncryptedDocumentException;
+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.ooxml.POIXMLDocumentPart;
 import org.apache.poi.ooxml.POIXMLDocumentPart.RelationPart;
@@ -55,6 +56,8 @@ import org.apache.poi.hssf.HSSFITestDataProvider;
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.ooxml.util.DocumentHelper;
+import org.apache.poi.ooxml.util.SAXHelper;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.exceptions.InvalidOperationException;
 import org.apache.poi.openxml4j.exceptions.OpenXML4JException;
@@ -104,6 +107,9 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedName;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedNames;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheet;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.impl.CTFontImpl;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXParseException;
+import org.xml.sax.XMLReader;
 
 public final class TestXSSFBugs extends BaseTestBugzillaIssues {
     public TestXSSFBugs() {
@@ -1915,6 +1921,36 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
         wb.close();
     }
 
+    @Test
+    public void test54764WithSAXHelper() throws Exception {
+        File testFile = XSSFTestDataSamples.getSampleFile("54764.xlsx");
+        ZipFile zip = new ZipFile(testFile);
+        ZipArchiveEntry ze = zip.getEntry("xl/sharedStrings.xml");
+        XMLReader reader = SAXHelper.newXMLReader();
+        try {
+            reader.parse(new InputSource(zip.getInputStream(ze)));
+            fail("should have thrown SAXParseException");
+        } catch (SAXParseException e) {
+            assertNotNull(e.getMessage());
+            assertTrue(e.getMessage().contains("more than \"1\" entity"));
+        }
+    }
+
+    @Test
+    public void test54764WithDocumentHelper() throws Exception {
+        File testFile = XSSFTestDataSamples.getSampleFile("54764.xlsx");
+        ZipFile zip = new ZipFile(testFile);
+        ZipArchiveEntry ze = zip.getEntry("xl/sharedStrings.xml");
+        try {
+            DocumentHelper.readDocument(zip.getInputStream(ze));
+            fail("should have thrown SAXParseException");
+        } catch (SAXParseException e) {
+            assertNotNull(e.getMessage());
+            e.printStackTrace();
+            assertTrue(e.getMessage().contains("more than \"1\" entity"));
+        }
+    }
+
     /**
      * CTDefinedNamesImpl should be included in the smaller
      * poi-ooxml-schemas jar