From: Nick Burch Date: Mon, 4 Aug 2014 18:17:26 +0000 (+0000) Subject: Apply suggestions from Uwe Schindler for more secure xml defaults for #54764 and... X-Git-Tag: REL_3_11_BETA2~12 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=526abfe065b91c8baa02fd92918604e024764b82;p=poi.git 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 --- 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 Binary files /dev/null and b/test-data/spreadsheet/54764.xlsx differ