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

3 files changed, 53 insertions, 1 deletions

diff --git a/src/ooxml/java/org/apache/poi/util/SAXHelper.java b/src/ooxml/java/org/apache/poi/util/SAXHelper.java
index 6a43292a53..b38b2c2be9 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 dd5e0505e9..a5420b0b79 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 0000000000..938245b61a
--- /dev/null
+++ b/test-data/spreadsheet/54764.xlsx