]> source.dussan.org Git - poi.git/commitdiff
[bug-63005] disable the reading of DTDs altogether
authorPJ Fanning <fanningpj@apache.org>
Mon, 17 Dec 2018 19:06:31 +0000 (19:06 +0000)
committerPJ Fanning <fanningpj@apache.org>
Mon, 17 Dec 2018 19:06:31 +0000 (19:06 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1849123 13f79535-47bb-0310-9956-ffa450edef68

src/ooxml/java/org/apache/poi/ooxml/util/DocumentHelper.java
src/ooxml/testcases/org/apache/poi/ooxml/util/TestDocumentHelper.java
src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestContentType.java
src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackageCoreProperties.java
src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestRelationships.java
src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java

index 6b0bd558408762f701c95da5e7cb651611c1f3f8..cf9fbf71ad634eabaa7474187d6b761a51b346ba 100644 (file)
@@ -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);
index febec72b5f9980a0c88c1b8ea6ea1e6344e0065a..63c6610f7e1f20d20f04a13fbb56d07fff3c4b64 100644 (file)
@@ -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) {
index 91187ceb9f6221f0aef4573cea08dd06e7c20de4..7a0d0369ece67b43809060ba774a0b447fc8b6ef 100644 (file)
@@ -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/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.
+     * <p>
+     * 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/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);
index c4900484429e747e7d5ac5a01cf135e5a5ddf728..17bf1828af2fd7f4df28cbb761beb624b967111a 100644 (file)
@@ -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();
     }
index ebbc309ffa660d563e9c10651c30a733f2eb9a10..846eac2a1ab3417958050e20eefb6535ba2fb5dd 100644 (file)
@@ -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);
     }
 }
index ef83956468f3aae5d7663ee979f146b0e1d7066f..379109303b81494623f934a4442e9e34c2c7ee41 100644 (file)
@@ -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"));
         }
     }