From: PJ Fanning Date: Mon, 17 Dec 2018 19:06:31 +0000 (+0000) Subject: [bug-63005] disable the reading of DTDs altogether X-Git-Tag: REL_4_1_0~179 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d1400559f88c97cdc71cb263d6b66aa547903167;p=poi.git [bug-63005] disable the reading of DTDs altogether git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1849123 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 6b0bd55840..cf9fbf71ad 100644 --- a/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java +++ b/src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java @@ -105,6 +105,7 @@ public final class DocumentHelper { //entity expansions to 1 in trySetXercesSecurityManager documentBuilderFactory.setExpandEntityReferences(false); trySetFeature(documentBuilderFactory, XMLConstants.FEATURE_SECURE_PROCESSING, true); + trySetFeature(documentBuilderFactory, POIXMLConstants.FEATURE_DISALLOW_DOCTYPE_DECL, true); trySetFeature(documentBuilderFactory, POIXMLConstants.FEATURE_LOAD_DTD_GRAMMAR, false); trySetFeature(documentBuilderFactory, POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD, false); trySetXercesSecurityManager(documentBuilderFactory); diff --git a/src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java index febec72b5f..63c6610f7e 100644 --- a/src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java +++ b/src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java @@ -39,6 +39,7 @@ public class TestDocumentHelper { public void testDocumentBuilderFactory() throws Exception { try { assertTrue(DocumentHelper.documentBuilderFactory.getFeature(XMLConstants.FEATURE_SECURE_PROCESSING)); + assertTrue(DocumentHelper.documentBuilderFactory.getFeature(POIXMLConstants.FEATURE_DISALLOW_DOCTYPE_DECL)); assertFalse(DocumentHelper.documentBuilderFactory.getFeature(POIXMLConstants.FEATURE_LOAD_DTD_GRAMMAR)); assertFalse(DocumentHelper.documentBuilderFactory.getFeature(POIXMLConstants.FEATURE_LOAD_EXTERNAL_DTD)); } catch(AbstractMethodError e) { diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java index 91187ceb9f..7a0d0369ec 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java @@ -19,163 +19,150 @@ package org.apache.poi.openxml4j.opc; import java.io.InputStream; -import junit.framework.TestCase; - import org.apache.poi.openxml4j.OpenXML4JTestDataSamples; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.opc.internal.ContentType; -import org.apache.poi.xwpf.usermodel.XWPFRelation; +import org.junit.Test; + +import static org.junit.Assert.*; /** * Tests for content type (ContentType class). * * @author Julien Chable */ -public final class TestContentType extends TestCase { - - /** - * Check rule M1.13: Package implementers shall only create and only - * recognize parts with a content type; format designers shall specify a - * content type for each part included in the format. Content types for - * package parts shall fit the definition and syntax for media types as - * specified in RFC 2616, \u00A73.7. - */ - public void testContentTypeValidation() throws InvalidFormatException { - String[] contentTypesToTest = new String[] { "text/xml", - "application/pgp-key", "application/vnd.hp-PCLXL", - "application/vnd.lotus-1-2-3" }; - for (String contentType : contentTypesToTest) { - new ContentType(contentType); - } - } - - /** - * Check rule M1.13 : Package implementers shall only create and only - * recognize parts with a content type; format designers shall specify a - * content type for each part included in the format. Content types for - * package parts shall fit the definition and syntax for media types as - * specified in RFC 2616, \u00A3.7. - * - * Check rule M1.14: Content types shall not use linear white space either - * between the type and subtype or between an attribute and its value. - * Content types also shall not have leading or trailing white spaces. - * Package implementers shall create only such content types and shall - * require such content types when retrieving a part from a package; format - * designers shall specify only such content types for inclusion in the - * format. - */ - public void testContentTypeValidationFailure() { - String[] contentTypesToTest = new String[] { "text/xml/app", "", - "test", "text(xml/xml", "text)xml/xml", "text/xml", "text@/xml", "text,/xml", "text;/xml", - "text:/xml", "text\\/xml", "t/ext/xml", "t\"ext/xml", - "text[/xml", "text]/xml", "text?/xml", "tex=t/xml", - "te{xt/xml", "tex}t/xml", "te xt/xml", - "text" + (char) 9 + "/xml", "text xml", " text/xml " }; - for (String contentType : contentTypesToTest) { - try { - new ContentType(contentType); - } catch (InvalidFormatException e) { - continue; - } - fail("Must have fail for content type: '" + contentType + "' !"); - } - } - - /** - * Parameters are allowed, provides that they meet the - * criteria of rule [01.2] - * Invalid parameters are verified as incorrect in - * {@link #testContentTypeParameterFailure()} - */ - public void testContentTypeParam() throws InvalidFormatException { - String[] contentTypesToTest = new String[] { "mail/toto;titi=tata", - "text/xml;a=b;c=d", "text/xml;key1=param1;key2=param2", - "application/pgp-key;version=\"2\"", - "application/x-resqml+xml;version=2.0;type=obj_global2dCrs" - }; - for (String contentType : contentTypesToTest) { - new ContentType(contentType); - } - } - - /** - * Check rule [O1.2]: Format designers might restrict the usage of - * parameters for content types. - */ - public void testContentTypeParameterFailure() { - String[] contentTypesToTest = new String[] { - "mail/toto;\"titi=tata\"", // quotes not allowed like that +public final class TestContentType { + + /** + * Check rule M1.13: Package implementers shall only create and only + * recognize parts with a content type; format designers shall specify a + * content type for each part included in the format. Content types for + * package parts shall fit the definition and syntax for media types as + * specified in RFC 2616, \u00A73.7. + */ + @Test + public void testContentTypeValidation() throws InvalidFormatException { + String[] contentTypesToTest = new String[]{"text/xml", + "application/pgp-key", "application/vnd.hp-PCLXL", + "application/vnd.lotus-1-2-3"}; + for (String contentType : contentTypesToTest) { + new ContentType(contentType); + } + } + + /** + * Check rule M1.13 : Package implementers shall only create and only + * recognize parts with a content type; format designers shall specify a + * content type for each part included in the format. Content types for + * package parts shall fit the definition and syntax for media types as + * specified in RFC 2616, \u00A3.7. + *

+ * Check rule M1.14: Content types shall not use linear white space either + * between the type and subtype or between an attribute and its value. + * Content types also shall not have leading or trailing white spaces. + * Package implementers shall create only such content types and shall + * require such content types when retrieving a part from a package; format + * designers shall specify only such content types for inclusion in the + * format. + */ + @Test + public void testContentTypeValidationFailure() { + String[] contentTypesToTest = new String[]{"text/xml/app", "", + "test", "text(xml/xml", "text)xml/xml", "text/xml", "text@/xml", "text,/xml", "text;/xml", + "text:/xml", "text\\/xml", "t/ext/xml", "t\"ext/xml", + "text[/xml", "text]/xml", "text?/xml", "tex=t/xml", + "te{xt/xml", "tex}t/xml", "te xt/xml", + "text" + (char) 9 + "/xml", "text xml", " text/xml "}; + for (String contentType : contentTypesToTest) { + try { + new ContentType(contentType); + } catch (InvalidFormatException e) { + continue; + } + fail("Must have fail for content type: '" + contentType + "' !"); + } + } + + /** + * Parameters are allowed, provides that they meet the + * criteria of rule [01.2] + * Invalid parameters are verified as incorrect in + * {@link #testContentTypeParameterFailure()} + */ + @Test + public void testContentTypeParam() throws InvalidFormatException { + String[] contentTypesToTest = new String[]{"mail/toto;titi=tata", + "text/xml;a=b;c=d", "text/xml;key1=param1;key2=param2", + "application/pgp-key;version=\"2\"", + "application/x-resqml+xml;version=2.0;type=obj_global2dCrs" + }; + for (String contentType : contentTypesToTest) { + new ContentType(contentType); + } + } + + /** + * Check rule [O1.2]: Format designers might restrict the usage of + * parameters for content types. + */ + @Test + public void testContentTypeParameterFailure() { + String[] contentTypesToTest = new String[]{ + "mail/toto;\"titi=tata\"", // quotes not allowed like that "mail/toto;titi = tata", // spaces not allowed "text/\u0080" // characters above ASCII are not allowed }; - for (String contentType : contentTypesToTest) { - try { - new ContentType(contentType); - } catch (InvalidFormatException e) { - continue; - } - fail("Must have fail for content type: '" + contentType + "' !"); - } - } - - /** - * Check rule M1.15: The package implementer shall require a content type - * that does not include comments and the format designer shall specify such - * a content type. - */ - public void testContentTypeCommentFailure() { - String[] contentTypesToTest = new String[] { "text/xml(comment)" }; - for (String contentType : contentTypesToTest) { - try { - new ContentType(contentType); - } catch (InvalidFormatException e) { - continue; - } - fail("Must have fail for content type: '" + contentType + "' !"); - } - } - - /** - * OOXML content types don't need entities, but we shouldn't - * barf if we get one from a third party system that added them - */ - public void testFileWithContentTypeEntities() throws Exception { - InputStream is = OpenXML4JTestDataSamples.openSampleStream("ContentTypeHasEntities.ooxml"); - OPCPackage p = OPCPackage.open(is); - - // Check we found the contents of it - boolean foundCoreProps = false, foundDocument = false, foundTheme1 = false; - for (PackagePart part : p.getParts()) { - if (part.getPartName().toString().equals("/docProps/core.xml")) { - assertEquals(ContentTypes.CORE_PROPERTIES_PART, part.getContentType()); - foundCoreProps = true; + for (String contentType : contentTypesToTest) { + try { + new ContentType(contentType); + } catch (InvalidFormatException e) { + continue; } - if (part.getPartName().toString().equals("/word/document.xml")) { - assertEquals(XWPFRelation.DOCUMENT.getContentType(), part.getContentType()); - foundDocument = true; - } - if (part.getPartName().toString().equals("/word/theme/theme1.xml")) { - assertEquals(XWPFRelation.THEME.getContentType(), part.getContentType()); - foundTheme1 = true; + fail("Must have fail for content type: '" + contentType + "' !"); + } + } + + /** + * Check rule M1.15: The package implementer shall require a content type + * that does not include comments and the format designer shall specify such + * a content type. + */ + @Test + public void testContentTypeCommentFailure() { + String[] contentTypesToTest = new String[]{"text/xml(comment)"}; + for (String contentType : contentTypesToTest) { + try { + new ContentType(contentType); + } catch (InvalidFormatException e) { + continue; } + fail("Must have fail for content type: '" + contentType + "' !"); } - assertTrue("Core not found in " + p.getParts(), foundCoreProps); - assertTrue("Document not found in " + p.getParts(), foundDocument); - assertTrue("Theme1 not found in " + p.getParts(), foundTheme1); - } - - /** - * Check that we can open a file where there are valid - * parameters on a content type - */ - public void testFileWithContentTypeParams() throws Exception { + } + + /** + * OOXML content types don't need entities and we shouldn't + * barf if we get one from a third party system that added them + */ + @Test(expected = InvalidFormatException.class) + public void testFileWithContentTypeEntities() throws Exception { + InputStream is = OpenXML4JTestDataSamples.openSampleStream("ContentTypeHasEntities.ooxml"); + OPCPackage.open(is); + } + + /** + * Check that we can open a file where there are valid + * parameters on a content type + */ + @Test + public void testFileWithContentTypeParams() throws Exception { InputStream is = OpenXML4JTestDataSamples.openSampleStream("ContentTypeHasParameters.ooxml"); OPCPackage p = OPCPackage.open(is); - + final String typeResqml = "application/x-resqml+xml"; - + // Check the types on everything for (PackagePart part : p.getParts()) { final String contentType = part.getContentType(); @@ -207,8 +194,7 @@ public final class TestContentType extends TestCase { assertEquals(2, length); assertEquals("2.0", details.getParameter("version")); assertEquals("obj_global1dCrs", details.getParameter("type")); - } - else if (part.getPartName().toString().equals("/global2dCrs.xml")) { + } else if (part.getPartName().toString().equals("/global2dCrs.xml")) { assertTrue(part.getContentType().startsWith(typeResqml)); assertEquals(typeResqml, details.toString(false)); assertEquals(true, hasParameters); diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackageCoreProperties.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackageCoreProperties.java index c490048442..17bf1828af 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackageCoreProperties.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackageCoreProperties.java @@ -249,8 +249,8 @@ public final class TestPackageCoreProperties { // Get the Core Properties PackagePropertiesPart props = (PackagePropertiesPart)p.getPackageProperties(); - // Check - assertEquals("Stefan Kopf", props.getCreatorProperty().get()); + // used to resolve a vale but now we ignore DTD entities for security reasons + assertFalse(props.getCreatorProperty().isPresent()); p.close(); } diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestRelationships.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestRelationships.java index ebbc309ffa..846eac2a1a 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestRelationships.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestRelationships.java @@ -418,28 +418,8 @@ public class TestRelationships extends TestCase { if (pr.getRelationshipType().equals(PackageRelationshipTypes.EXTENDED_PROPERTIES)) foundExtPropRel = true; } - assertTrue("Core/Doc Relationship not found in " + p.getRelationships(), foundDocRel); - assertTrue("Core Props Relationship not found in " + p.getRelationships(), foundCorePropRel); - assertTrue("Ext Props Relationship not found in " + p.getRelationships(), foundExtPropRel); - - // Should have normal work parts - boolean foundCoreProps = false, foundDocument = false, foundTheme1 = false; - for (PackagePart part : p.getParts()) { - if (part.getPartName().toString().equals("/docProps/core.xml")) { - assertEquals(ContentTypes.CORE_PROPERTIES_PART, part.getContentType()); - foundCoreProps = true; - } - if (part.getPartName().toString().equals("/word/document.xml")) { - assertEquals(XWPFRelation.DOCUMENT.getContentType(), part.getContentType()); - foundDocument = true; - } - if (part.getPartName().toString().equals("/word/theme/theme1.xml")) { - assertEquals(XWPFRelation.THEME.getContentType(), part.getContentType()); - foundTheme1 = true; - } - } - assertTrue("Core not found in " + p.getParts(), foundCoreProps); - assertTrue("Document not found in " + p.getParts(), foundDocument); - assertTrue("Theme1 not found in " + p.getParts(), foundTheme1); + assertFalse("Core/Doc Relationship not found in " + p.getRelationships(), foundDocRel); + assertFalse("Core Props Relationship not found in " + p.getRelationships(), foundCorePropRel); + assertFalse("Ext Props Relationship not found in " + p.getRelationships(), foundExtPropRel); } } 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 ef83956468..379109303b 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -1969,7 +1969,7 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { fail("should have thrown SAXParseException"); } catch (SAXParseException e) { assertNotNull(e.getMessage()); - assertTrue(e.getMessage().contains("more than \"1\" entity")); + assertTrue(e.getMessage().contains("DOCTYPE is disallowed when the feature")); } }