secure xml

bw-webdav.iml

@@ -5,10 +5,16 @@
     <output-test url="file://$MODULE_DIR$/target/test-classes" />
     <content url="file://$MODULE_DIR$">
       <sourceFolder url="file://$MODULE_DIR$/src/main/java" isTestSource="false" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/java" isTestSource="true" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/resources" type="java-test-resource" />
       <excludeFolder url="file://$MODULE_DIR$/target" />
     </content>
     <orderEntry type="inheritedJdk" />
     <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="library" scope="TEST" name="Maven: junit:junit:4.12" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.hamcrest:hamcrest-core:1.3" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.mockito:mockito-core:1.10.19" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.objenesis:objenesis:2.1" level="project" />
     <orderEntry type="library" name="Maven: org.bedework:bw-util-misc:4.0.18" level="project" />
     <orderEntry type="library" name="Maven: org.bedework:bw-util-xml:4.0.18" level="project" />
     <orderEntry type="library" name="Maven: org.bedework:bw-util-servlet:4.0.18" level="project" />
@@ -19,7 +25,6 @@
     <orderEntry type="library" name="Maven: com.fasterxml.jackson.core:jackson-core:2.3.1" level="project" />
     <orderEntry type="library" name="Maven: org.bedework:bw-access:4.0.2" level="project" />
     <orderEntry type="library" name="Maven: org.bedework:bw-util-caching:4.0.18" level="project" />
-    <orderEntry type="library" name="Maven: junit:junit:4.8.2" level="project" />
     <orderEntry type="library" name="Maven: log4j:log4j:1.2.15" level="project" />
     <orderEntry type="library" name="Maven: javax.mail:mail:1.4" level="project" />
     <orderEntry type="library" name="Maven: javax.activation:activation:1.1" level="project" />

mvnvm.properties

@@ -0,0 +1 @@
+mvn_version=3.5.4

pom.xml

@@ -85,6 +85,18 @@
   </distributionManagement>
 
 	<dependencies>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.12</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>1.10.19</version>
+            <scope>test</scope>
+        </dependency>
 		<!-- Bedework Dependencies -->
     <dependency>
       <groupId>org.bedework</groupId>

src/main/java/org/bedework/webdav/servlet/common/MethodBase.java

@@ -56,7 +56,7 @@
 
 /** Base class for all webdav servlet methods.
  */
-public abstract class MethodBase extends Logged {
+public abstract class MethodBase extends Logged implements SecureXml {
   protected boolean dumpContent;
 
   protected boolean hasBriefHeader;
@@ -358,34 +358,13 @@ protected Document parseContent(final HttpServletRequest req,
 
   /** Parse a reader and return the DOM representation.
    *
-   * @param len        Content length
-   * @param rdr        Reader
+   * @param contentLength        Content length
+   * @param reader        Reader
    * @return Document  Parsed body or null for no body
    * @exception WebdavException Some error occurred.
    */
-  protected Document parseContent(final int len,
-                                  final Reader rdr) throws WebdavException{
-    if (len == 0) {
-      return null;
-    }
-
-    if (rdr == null) {
-      // No content?
-      return null;
-    }
-
-    try {
-      DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-      factory.setNamespaceAware(true);
-
-      DocumentBuilder builder = factory.newDocumentBuilder();
-
-      return builder.parse(new InputSource(rdr));
-    } catch (SAXException e) {
-      throw new WebdavBadRequest();
-    } catch (Throwable t) {
-      throw new WebdavException(t);
-    }
+  protected Document parseContent(final int contentLength, final Reader reader) throws WebdavException{
+    return parseXmlSafely(contentLength, reader);
   }
 
   protected String formatHTTPDate(final Timestamp val) {

src/main/java/org/bedework/webdav/servlet/common/PostRequestPars.java

@@ -19,23 +19,18 @@
 package org.bedework.webdav.servlet.common;
 
 import org.bedework.util.misc.Util;
-import org.bedework.webdav.servlet.shared.WebdavBadRequest;
 import org.bedework.webdav.servlet.shared.WebdavException;
 import org.bedework.webdav.servlet.shared.WebdavNsIntf;
 
 import org.w3c.dom.Document;
-import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
 
 import java.io.Reader;
 
 import javax.servlet.http.HttpServletRequest;
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
 
 /**
  */
-public class PostRequestPars {
+public class PostRequestPars implements SecureXml {
   private final HttpServletRequest req;
 
   private final WebdavNsIntf intf;
@@ -126,7 +121,7 @@ public boolean processXml() throws WebdavException {
       throw new WebdavException(t);
     }
 
-    xmlDoc = parseXml(reqRdr);
+    xmlDoc = parseXmlSafely(req.getContentLength(), reqRdr);
     getTheReader = false;
     return true;
   }
@@ -216,31 +211,7 @@ public Document getXmlDoc() {
   public boolean isAddMember() {
     return addMember;
   }
-
-  /**
-   * @param rdr for xml content
-   * @return parsed Document
-   * @throws WebdavException
-   */
-  private Document parseXml(final Reader rdr) throws WebdavException{
-    if (rdr == null) {
-      return null;
-    }
-
-    try {
-      final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-      factory.setNamespaceAware(true);
-
-      final DocumentBuilder builder = factory.newDocumentBuilder();
-
-      return builder.parse(new InputSource(rdr));
-    } catch (final SAXException e) {
-      throw new WebdavBadRequest();
-    } catch (final Throwable t) {
-      throw new WebdavException(t);
-    }
-  }
-
+
   /**
    * @return true if we have an xml content
    */

src/main/java/org/bedework/webdav/servlet/common/SecureXml.java

@@ -0,0 +1,42 @@
+package org.bedework.webdav.servlet.common;
+
+import org.bedework.webdav.servlet.shared.WebdavBadRequest;
+import org.bedework.webdav.servlet.shared.WebdavException;
+import org.w3c.dom.Document;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import java.io.Reader;
+
+public interface SecureXml {
+
+  default Document parseXmlSafely(final int contentLength, final Reader reader) throws WebdavException {
+    if (contentLength == 0) {
+      return null;
+    }
+
+    if (reader == null) {
+      // No content?
+      return null;
+    }
+
+    try {
+      DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+      factory.setNamespaceAware(false);
+      factory.setFeature("http://javax.xml.XMLConstants/feature/secure-processing", true);
+      factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+      factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+      factory.setAttribute("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+
+      DocumentBuilder builder = factory.newDocumentBuilder();
+
+      return builder.parse(new InputSource(reader));
+    } catch (SAXException exception) {
+      throw new WebdavBadRequest(exception.getMessage());
+    } catch (Throwable t) {
+      throw new WebdavException(t);
+    }
+  }
+}

src/test/java/org/bedework/webdav/servlet/common/TestSecureXmlTypes.java

@@ -0,0 +1,68 @@
+package org.bedework.webdav.servlet.common;
+
+import org.bedework.webdav.servlet.shared.WebdavException;
+import org.bedework.webdav.servlet.shared.WebdavNsIntf;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.UUID;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
+
+public class TestSecureXmlTypes {
+
+  @Rule
+  public MockitoRule mockitoRule = MockitoJUnit.rule();
+
+  @Mock
+  private HttpServletRequest request;
+  @Mock
+  private HttpServletResponse response;
+
+  @Mock(answer = Answers.CALLS_REAL_METHODS)
+  private WebdavNsIntf webdavNsIntf;
+
+  @Mock(answer = Answers.CALLS_REAL_METHODS)
+  private MethodBase methodBase;
+
+  private PostRequestPars requestPars;
+
+  @Before
+  public void setup() throws WebdavException, IOException {
+    when(request.getContentType()).thenReturn("application/xml");
+    requestPars = new PostRequestPars(request, webdavNsIntf, UUID.randomUUID().toString());
+
+    when(methodBase.getNsIntf()).thenReturn(webdavNsIntf);
+
+    when(request.getContentLength()).thenReturn(1);
+    when(request.getReader()).thenReturn(getResource("/malicious-request.xml"));
+  }
+
+  @Test
+  public void testPostRequestParsWithMaliciousRequest() throws WebdavException {
+    assertTrue(requestPars.processXml());
+  }
+
+  @Test
+  public void testMethodBaseWithMaliciousRequest() throws WebdavException {
+    assertNotNull(methodBase.parseContent(request, response));
+  }
+
+  private BufferedReader getResource(final String name) {
+    return new BufferedReader(
+        new InputStreamReader(this.getClass().getResourceAsStream(name))
+    );
+  }
+}

src/test/resources/malicious-request.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE invite-reply [
+        <!ENTITY xxe SYSTEM "file:///xxe">
+        ]>
+<A:invite-reply xmlns:A="http://calendarserver.org/ns/" xmlns:B="DAV:">
+    <A:invite-accepted>&xxe;</A:invite-accepted>
+    <B:href>/some/url</B:href>
+    <A:in-reply-to>something</A:in-reply-to>
+    <A:hosturl>
+        <B:href>/</B:href>
+    </A:hosturl>
+</A:invite-reply>