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