You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ch...@apache.org on 2017/09/01 18:17:20 UTC

svn commit: r1806999 - in /commons/proper/jelly/trunk/core/src: main/java/org/apache/commons/jelly/ main/java/org/apache/commons/jelly/parser/ test/java/org/apache/commons/jelly/ test/resources/org/apache/commons/jelly/

Author: chtompki
Date: Fri Sep  1 18:17:20 2017
New Revision: 1806999

URL: http://svn.apache.org/viewvc?rev=1806999&view=rev
Log:
JELLY-293: Accommodate toggling off DTD external entities.

Added:
    commons/proper/jelly/trunk/core/src/test/java/org/apache/commons/jelly/TestDoctypeDefinitionXXE.java
    commons/proper/jelly/trunk/core/src/test/resources/org/apache/commons/jelly/doctypeDefinitionXXE.jelly
Modified:
    commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/JellyContext.java
    commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java

Modified: commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/JellyContext.java
URL: http://svn.apache.org/viewvc/commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/JellyContext.java?rev=1806999&r1=1806998&r2=1806999&view=diff
==============================================================================
--- commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/JellyContext.java (original)
+++ commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/JellyContext.java Fri Sep  1 18:17:20 2017
@@ -50,6 +50,9 @@ public class JellyContext {
     /** Default for export of variables **/
     private static final boolean DEFAULT_EXPORT = false;
 
+    /** Default for DTD calling out to external entities. */
+    private static final boolean DEFAULT_ALLOW_DTD_CALLS_TO_EXTERNAL_ENTITIES = false;
+
     /** String used to denote a script can't be parsed */
     private static final String BAD_PARSE = "Could not parse Jelly script";
 
@@ -88,6 +91,9 @@ public class JellyContext {
     /** Do we export our variables to parent context? */
     private boolean export  = JellyContext.DEFAULT_EXPORT;
 
+    /** Do we allow our doctype definitions to call out to external entities? */
+    private boolean allowDtdToCallExternalEntities = JellyContext.DEFAULT_ALLOW_DTD_CALLS_TO_EXTERNAL_ENTITIES;
+
     /** Should we export tag libraries to our parents context */
     private boolean exportLibraries = true;
 
@@ -576,7 +582,7 @@ public class JellyContext {
      * is created - such as to overload what the default ExpressionFactory should be.
      */
     protected XMLParser createXMLParser() {
-        return new XMLParser();
+        return new XMLParser(allowDtdToCallExternalEntities);
     }
 
     /**
@@ -882,6 +888,20 @@ public class JellyContext {
         return this.inherit;
     }
 
+    /**
+     * Sets whether we should allow our doctype definitions to call out to external entities.
+     */
+    public void setAllowDtdToCallExternalEntities(boolean allowDtdToCallExternalEntities) {
+        this.allowDtdToCallExternalEntities = allowDtdToCallExternalEntities;
+    }
+
+    /**
+     * @return whether we should allow our doctype definitions to call out to external entities.
+     */
+    public boolean isAllowDtdToCallExternalEntities() {
+        return this.allowDtdToCallExternalEntities;
+    }
+
 
     /**
      * Return the class loader to be used for instantiating application objects

Modified: commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java
URL: http://svn.apache.org/viewvc/commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java?rev=1806999&r1=1806998&r2=1806999&view=diff
==============================================================================
--- commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java (original)
+++ commons/proper/jelly/trunk/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java Fri Sep  1 18:17:20 2017
@@ -101,6 +101,9 @@ public class XMLParser extends DefaultHa
     /** The current text buffer where non-custom tags get written */
     private StringBuffer textBuffer;
 
+    /** Do we allow our doctype definitions to call out to external entities? */
+    private boolean allowDtdToCallExternalEntities = false;
+
     /**
      * The class loader to use for instantiating application objects.
      * If not specified, the context class loader, or the class loader
@@ -187,6 +190,21 @@ public class XMLParser extends DefaultHa
     }
 
     /**
+     * Construct a new XMLParser, with the boolean
+     * allowDtdToCallExternalEntities being passed in. If this is set to false,
+     * the XMLParser will be created with:
+     * XMLReader spf = XMLReaderFactory.createXMLReader();
+     * spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+     * spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+     * spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd",false);
+     * as given by
+     * https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet#XMLReader
+     */
+    public XMLParser(boolean allowDtdToCallExternalEntities) {
+        this.allowDtdToCallExternalEntities = allowDtdToCallExternalEntities;
+    }
+
+    /**
      * Construct a new XMLParser, allowing a SAXParser to be passed in.  This
      * allows XMLParser to be used in environments which are unfriendly to
      * JAXP1.1 (such as WebLogic 6.0).  Thanks for the request to change go to
@@ -546,6 +564,11 @@ public class XMLParser extends DefaultHa
     public synchronized XMLReader getXMLReader() throws SAXException {
         if (reader == null) {
             reader = getParser().getXMLReader();
+            if (!allowDtdToCallExternalEntities) {
+                reader.setFeature("http://xml.org/sax/features/external-general-entities", false);
+                reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+                reader.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            }
             if (this.defaultNamespaceURI != null) {
                 reader = new DefaultNamespaceFilter(this.defaultNamespaceURI,reader);
             }
@@ -614,6 +637,31 @@ public class XMLParser extends DefaultHa
     }
 
     /**
+     * Gets the internal state for allowance for DTDs to call external resources.
+     *
+     * @return true if we are allowed to make external calls to entities during XML
+     * parsing by custom declared resources in the DTD.
+     */
+    public boolean isAllowDtdToCallExternalEntities() {
+        return allowDtdToCallExternalEntities;
+    }
+
+    /**
+     * Sets the boolean
+     * allowDtdToCallExternalEntities. If this is set to false,
+     * the XMLParser will be created with:
+     * XMLReader spf = XMLReaderFactory.createXMLReader();
+     * spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+     * spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+     * spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd",false);
+     * as given by
+     * https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet#XMLReader
+     */
+    public void setAllowDtdToCallExternalEntities(boolean allowDtdToCallExternalEntities) {
+        this.allowDtdToCallExternalEntities = allowDtdToCallExternalEntities;
+    }
+
+    /**
      * Process notification of the start of an XML element being reached.
      *
      * @param namespaceURI The Namespace URI, or the empty string if the

Added: commons/proper/jelly/trunk/core/src/test/java/org/apache/commons/jelly/TestDoctypeDefinitionXXE.java
URL: http://svn.apache.org/viewvc/commons/proper/jelly/trunk/core/src/test/java/org/apache/commons/jelly/TestDoctypeDefinitionXXE.java?rev=1806999&view=auto
==============================================================================
--- commons/proper/jelly/trunk/core/src/test/java/org/apache/commons/jelly/TestDoctypeDefinitionXXE.java (added)
+++ commons/proper/jelly/trunk/core/src/test/java/org/apache/commons/jelly/TestDoctypeDefinitionXXE.java Fri Sep  1 18:17:20 2017
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jelly;
+
+import junit.framework.TestCase;
+
+import java.net.URL;
+
+/**
+ * A test class to validate doctype definitions' declaration of external
+ * calls using custom xml tags. Specifically we test some changes in {@link JellyContext}
+ * along with {@link org.apache.commons.jelly.parser.XMLParser}.
+ *
+ * @author chotmpki
+ */
+public class TestDoctypeDefinitionXXE extends TestCase
+{
+    public TestDoctypeDefinitionXXE( String s )
+    {
+        super( s );
+    }
+
+    public void testDoctypeDefinitionXXEDefaultMode() throws JellyException
+    {
+        JellyContext context = new JellyContext();
+        URL url = this.getClass().getResource("doctypeDefinitionXXE.jelly");
+        try
+        {
+            context.runScript(url, null);
+        } catch (JellyException e) {
+            Throwable cause = e.getCause();
+            if (cause instanceof java.net.ConnectException) {
+                fail("doctypeDefinitionXXE.jelly attempted to connect to http://127.0.0.1:4444");
+            } else if (cause instanceof org.xml.sax.SAXParseException) {
+                // Success.
+            } else {
+                fail("Unknown exception: " + e.getMessage());
+            }
+        }
+    }
+
+    public void testDoctypeDefinitionXXEAllowDTDCalls() throws JellyException
+    {
+        JellyContext context = new JellyContext();
+        context.setAllowDtdToCallExternalEntities(true);
+        URL url = this.getClass().getResource("doctypeDefinitionXXE.jelly");
+        try
+        {
+            context.runScript(url, null);
+        } catch (JellyException e) {
+            Throwable cause = e.getCause();
+            if (cause instanceof java.net.ConnectException) {
+                //success
+            } else if (cause instanceof org.xml.sax.SAXParseException) {
+                fail("doctypeDefinitionXXE.jelly did not attempt to connect to http://127.0.0.1:4444");
+            } else {
+                fail("Unknown exception: " + e.getMessage());
+            }
+        }
+    }
+}
\ No newline at end of file

Added: commons/proper/jelly/trunk/core/src/test/resources/org/apache/commons/jelly/doctypeDefinitionXXE.jelly
URL: http://svn.apache.org/viewvc/commons/proper/jelly/trunk/core/src/test/resources/org/apache/commons/jelly/doctypeDefinitionXXE.jelly?rev=1806999&view=auto
==============================================================================
--- commons/proper/jelly/trunk/core/src/test/resources/org/apache/commons/jelly/doctypeDefinitionXXE.jelly (added)
+++ commons/proper/jelly/trunk/core/src/test/resources/org/apache/commons/jelly/doctypeDefinitionXXE.jelly Fri Sep  1 18:17:20 2017
@@ -0,0 +1,24 @@
+<?xml version="1.0"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+      http://www.apache.org/licenses/LICENSE-2.0
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!DOCTYPE r [
+        <!ELEMENT r ANY >
+        <!ENTITY sp SYSTEM "http://127.0.0.1:4444/">
+        ]>
+<r>&sp;</r>
+<j:jelly trim="false" xmlns:j="jelly:core"
+         xmlns:x="jelly:xml"
+         xmlns:html="jelly:html">
+</j:jelly>
\ No newline at end of file