This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5252-external in repository https://gitbox.apache.org/repos/asf/struts.git
commit 6658c6360e771a793ab261e5b4d3ed9dfb6720d3 Author: Lukasz Lenart <[email protected]> AuthorDate: Thu Oct 27 13:35:10 2022 +0200 WW-5252 Disables parsing external entities --- .../com/opensymphony/xwork2/util/DomHelper.java | 130 ++++++++++++--------- .../org/apache/struts2/views/xslt/XSLTResult.java | 60 +++++++--- core/src/main/resources/author.dtd | 22 ++++ .../opensymphony/xwork2/util/DomHelperTest.java | 56 ++++----- 4 files changed, 173 insertions(+), 95 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/util/DomHelper.java b/core/src/main/java/com/opensymphony/xwork2/util/DomHelper.java index f1021dc37..b79c3c03a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/DomHelper.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/DomHelper.java @@ -28,9 +28,17 @@ import org.apache.struts2.StrutsException; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; -import org.xml.sax.*; +import org.xml.sax.Attributes; +import org.xml.sax.ContentHandler; +import org.xml.sax.InputSource; +import org.xml.sax.Locator; +import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; +import org.xml.sax.SAXParseException; import org.xml.sax.helpers.DefaultHandler; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import javax.xml.transform.TransformerFactory; @@ -48,28 +56,24 @@ import java.util.Map; public class DomHelper { private static final Logger LOG = LogManager.getLogger(DomHelper.class); - - public static final String XMLNS_URI = "http://www.w3.org/2000/xmlns/";; public static Location getLocationObject(Element element) { return LocationAttributes.getLocation(element); } - /** * Creates a W3C Document that remembers the location of each element in * the source file. The location of element nodes can then be retrieved * using the {@link #getLocationObject(Element)} method. * * @param inputSource the inputSource to read the document from - * * @return the W3C Document */ public static Document parse(InputSource inputSource) { return parse(inputSource, null); } - - + + /** * Creates a W3C Document that remembers the location of each element in * the source file. The location of element nodes can then be retrieved @@ -77,17 +81,16 @@ public class DomHelper { * * @param inputSource the inputSource to read the document from * @param dtdMappings a map of DTD names and public ids - * * @return the W3C Document */ public static Document parse(InputSource inputSource, Map<String, String> dtdMappings) { - + SAXParserFactory factory = null; String parserProp = System.getProperty("xwork.saxParserFactory"); if (parserProp != null) { try { ObjectFactory objectFactory = ActionContext.getContext().getContainer().getInstance(ObjectFactory.class); - Class clazz = objectFactory.getClassInstance(parserProp); + Class<?> clazz = objectFactory.getClassInstance(parserProp); factory = (SAXParserFactory) clazz.newInstance(); } catch (Exception e) { LOG.error("Unable to load saxParserFactory set by system property 'xwork.saxParserFactory': {}", parserProp, e); @@ -98,6 +101,13 @@ public class DomHelper { factory = SAXParserFactory.newInstance(); } + try { + factory.setFeature("http://xml.org/sax/features/external-general-entities";, false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities";, false); + } catch (ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) { + throw new StrutsException("Unable to disable resolving external entities!", e); + } + factory.setValidating((dtdMappings != null)); factory.setNamespaceAware(true); @@ -107,22 +117,22 @@ public class DomHelper { } catch (Exception ex) { throw new StrutsException("Unable to create SAX parser", ex); } - - + + DOMBuilder builder = new DOMBuilder(); // Enhance the sax stream with location information ContentHandler locationHandler = new LocationAttributes.Pipe(builder); - + try { parser.parse(inputSource, new StartHandler(locationHandler, dtdMappings)); } catch (Exception ex) { throw new StrutsException(ex); } - + return builder.getDocument(); } - + /** * The <code>DOMBuilder</code> is a utility class that will generate a W3C * DOM Document from SAX events. @@ -130,27 +140,35 @@ public class DomHelper { * @author <a href="mailto:[email protected]";>Carsten Ziegeler</a> */ static public class DOMBuilder implements ContentHandler { - - /** The default transformer factory shared by all instances */ + + /** + * The default transformer factory shared by all instances + */ protected static SAXTransformerFactory FACTORY; - - /** The transformer factory */ + + /** + * The transformer factory + */ protected SAXTransformerFactory factory; - - /** The result */ + + /** + * The result + */ protected DOMResult result; - - /** The parentNode */ + + /** + * The parentNode + */ protected Node parentNode; - + protected ContentHandler nextHandler; - + static { String parserProp = System.getProperty("xwork.saxTransformerFactory"); if (parserProp != null) { try { ObjectFactory objectFactory = ActionContext.getContext().getContainer().getInstance(ObjectFactory.class); - Class clazz = objectFactory.getClassInstance(parserProp); + Class<?> clazz = objectFactory.getClassInstance(parserProp); FACTORY = (SAXTransformerFactory) clazz.newInstance(); } catch (Exception e) { LOG.error("Unable to load SAXTransformerFactory set by system property 'xwork.saxTransformerFactory': {}", parserProp, e); @@ -158,7 +176,7 @@ public class DomHelper { } if (FACTORY == null) { - FACTORY = (SAXTransformerFactory) TransformerFactory.newInstance(); + FACTORY = (SAXTransformerFactory) TransformerFactory.newInstance(); } } @@ -168,15 +186,16 @@ public class DomHelper { public DOMBuilder() { this((Node) null); } - + /** * Construct a new instance of this DOMBuilder. + * * @param factory the SAX transformer factory */ public DOMBuilder(SAXTransformerFactory factory) { this(factory, null); } - + /** * Constructs a new instance that appends nodes to the given parent node. * @@ -185,19 +204,19 @@ public class DomHelper { public DOMBuilder(Node parentNode) { this(null, parentNode); } - + /** * Construct a new instance of this DOMBuilder. * - * @param factory the SAX transformer factory + * @param factory the SAX transformer factory * @param parentNode the parent node */ public DOMBuilder(SAXTransformerFactory factory, Node parentNode) { - this.factory = factory == null? FACTORY: factory; + this.factory = factory == null ? FACTORY : factory; this.parentNode = parentNode; setup(); } - + /** * Setup this instance transformer and result objects. */ @@ -215,7 +234,7 @@ public class DomHelper { throw new StrutsException("Fatal-Error: Unable to get transformer handler", local); } } - + /** * Return the newly built Document. * @@ -230,60 +249,61 @@ public class DomHelper { return this.result.getNode().getOwnerDocument(); } } - + public void setDocumentLocator(Locator locator) { nextHandler.setDocumentLocator(locator); } - + public void startDocument() throws SAXException { nextHandler.startDocument(); } - + public void endDocument() throws SAXException { nextHandler.endDocument(); } - + public void startElement(String uri, String loc, String raw, Attributes attrs) throws SAXException { nextHandler.startElement(uri, loc, raw, attrs); } - + public void endElement(String arg0, String arg1, String arg2) throws SAXException { nextHandler.endElement(arg0, arg1, arg2); } - + public void startPrefixMapping(String arg0, String arg1) throws SAXException { nextHandler.startPrefixMapping(arg0, arg1); } - + public void endPrefixMapping(String arg0) throws SAXException { nextHandler.endPrefixMapping(arg0); } - + public void characters(char[] arg0, int arg1, int arg2) throws SAXException { nextHandler.characters(arg0, arg1, arg2); } - + public void ignorableWhitespace(char[] arg0, int arg1, int arg2) throws SAXException { nextHandler.ignorableWhitespace(arg0, arg1, arg2); } - + public void processingInstruction(String arg0, String arg1) throws SAXException { nextHandler.processingInstruction(arg0, arg1); } - + public void skippedEntity(String arg0) throws SAXException { nextHandler.skippedEntity(arg0); } } - + public static class StartHandler extends DefaultHandler { - - private ContentHandler nextHandler; - private Map<String, String> dtdMappings; - + + private final ContentHandler nextHandler; + private final Map<String, String> dtdMappings; + /** * Create a filter that is chained to another handler. - * @param next the next handler in the chain. + * + * @param next the next handler in the chain. * @param dtdMappings map of DTD mappings */ public StartHandler(ContentHandler next, Map<String, String> dtdMappings) { @@ -295,12 +315,12 @@ public class DomHelper { public void setDocumentLocator(Locator locator) { nextHandler.setDocumentLocator(locator); } - + @Override public void startDocument() throws SAXException { nextHandler.startDocument(); } - + @Override public void endDocument() throws SAXException { nextHandler.endDocument(); @@ -345,7 +365,7 @@ public class DomHelper { public void skippedEntity(String arg0) throws SAXException { nextHandler.skippedEntity(arg0); } - + @Override public InputSource resolveEntity(String publicId, String systemId) { if (dtdMappings != null && dtdMappings.containsKey(publicId)) { @@ -356,7 +376,7 @@ public class DomHelper { } return null; } - + @Override public void warning(SAXParseException exception) { } diff --git a/core/src/main/java/org/apache/struts2/views/xslt/XSLTResult.java b/core/src/main/java/org/apache/struts2/views/xslt/XSLTResult.java index d310a6db9..31690362c 100644 --- a/core/src/main/java/org/apache/struts2/views/xslt/XSLTResult.java +++ b/core/src/main/java/org/apache/struts2/views/xslt/XSLTResult.java @@ -18,7 +18,6 @@ */ package org.apache.struts2.views.xslt; -import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.Result; import com.opensymphony.xwork2.inject.Inject; @@ -32,7 +31,15 @@ import org.apache.struts2.StrutsConstants; import org.apache.struts2.StrutsException; import javax.servlet.http.HttpServletResponse; -import javax.xml.transform.*; +import javax.xml.XMLConstants; +import javax.xml.transform.ErrorListener; +import javax.xml.transform.OutputKeys; +import javax.xml.transform.Source; +import javax.xml.transform.Templates; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.URIResolver; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamSource; @@ -49,10 +56,14 @@ public class XSLTResult implements Result { private static final long serialVersionUID = 6424691441777176763L; - /** Log instance for this result. */ + /** + * Log instance for this result. + */ private static final Logger LOG = LogManager.getLogger(XSLTResult.class); - /** 'stylesheetLocation' parameter. Points to the xsl. */ + /** + * 'stylesheetLocation' parameter. Points to the xsl. + */ public static final String DEFAULT_PARAM = "stylesheetLocation"; /** @@ -66,22 +77,34 @@ public class XSLTResult implements Result { // Configurable Parameters - /** Determines whether or not the result should allow caching. */ + /** + * Determines whether or not the result should allow caching. + */ protected boolean noCache; - /** Indicates the location of the xsl template. */ + /** + * Indicates the location of the xsl template. + */ private String stylesheetLocation; - /** Indicates the property name patterns which should be exposed to the xml. */ + /** + * Indicates the property name patterns which should be exposed to the xml. + */ private String matchingPattern; - /** Indicates the property name patterns which should be excluded from the xml. */ + /** + * Indicates the property name patterns which should be excluded from the xml. + */ private String excludingPattern; - /** Indicates the ognl expression representing the bean which is to be exposed as xml. */ + /** + * Indicates the ognl expression representing the bean which is to be exposed as xml. + */ private String exposedValue; - /** Indicates the status to return in the response */ + /** + * Indicates the status to return in the response + */ private int status = 200; private String encoding = "UTF-8"; @@ -96,7 +119,7 @@ public class XSLTResult implements Result { this(); setStylesheetLocation(stylesheetLocation); } - + @Inject(StrutsConstants.STRUTS_XSLT_NOCACHE) public void setNoCache(String xsltNoCache) { this.noCache = BooleanUtils.toBoolean(xsltNoCache); @@ -124,7 +147,7 @@ public class XSLTResult implements Result { public void setStatus(String status) { try { - this.status = Integer.valueOf(status); + this.status = Integer.parseInt(status); } catch (NumberFormatException e) { throw new IllegalArgumentException("Status value not number " + e.getMessage(), e); } @@ -175,7 +198,8 @@ public class XSLTResult implements Result { templates = getTemplates(location); transformer = templates.newTransformer(); } else { - transformer = TransformerFactory.newInstance().newTransformer(); + TransformerFactory factory = createTransformerFactory(); + transformer = factory.newTransformer(); } transformer.setURIResolver(getURIResolver()); @@ -217,6 +241,14 @@ public class XSLTResult implements Result { } } + protected TransformerFactory createTransformerFactory() { + TransformerFactory factory = TransformerFactory.newInstance(); + LOG.debug("Disables parsing external entities"); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); + return factory; + } + protected ErrorListener buildErrorListener() { return new ErrorListener() { @@ -282,6 +314,6 @@ public class XSLTResult implements Result { } protected Source getDOMSourceForStack(Object value) throws IllegalAccessException, InstantiationException { - return new DOMSource(getAdapterFactory().adaptDocument("result", value) ); + return new DOMSource(getAdapterFactory().adaptDocument("result", value)); } } diff --git a/core/src/main/resources/author.dtd b/core/src/main/resources/author.dtd new file mode 100644 index 000000000..4521075a0 --- /dev/null +++ b/core/src/main/resources/author.dtd @@ -0,0 +1,22 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +/* + * 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. + */ +--> +<!ENTITY writer "Dock"> diff --git a/core/src/test/java/com/opensymphony/xwork2/util/DomHelperTest.java b/core/src/test/java/com/opensymphony/xwork2/util/DomHelperTest.java index 330dfe35f..4c56f5759 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/DomHelperTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/DomHelperTest.java @@ -32,42 +32,46 @@ import java.io.StringReader; */ public class DomHelperTest extends TestCase { - private String xml = "<!DOCTYPE foo [\n" + - "<!ELEMENT foo (bar)>\n" + - "<!ELEMENT bar (#PCDATA)>\n" + - "]>\n" + - "<foo>\n" + - " <bar/>\n" + - "</foo>\n"; - - public void testParse() throws Exception { + private final String xml = "<!DOCTYPE foo [<!ELEMENT foo (bar)><!ELEMENT bar (#PCDATA)>]>\n<foo>\n<bar/>\n</foo>\n"; + + public void testParse() { InputSource in = new InputSource(new StringReader(xml)); in.setSystemId("foo://bar"); - + Document doc = DomHelper.parse(in); assertNotNull(doc); - assertTrue("Wrong root node", - "foo".equals(doc.getDocumentElement().getNodeName())); - + assertEquals("Wrong root node", "foo", doc.getDocumentElement().getNodeName()); + NodeList nl = doc.getElementsByTagName("bar"); - assertTrue(nl.getLength() == 1); - - - + assertEquals(1, nl.getLength()); } - - public void testGetLocationObject() throws Exception { + + public void testGetLocationObject() { InputSource in = new InputSource(new StringReader(xml)); in.setSystemId("foo://bar"); - + Document doc = DomHelper.parse(in); - + NodeList nl = doc.getElementsByTagName("bar"); - - Location loc = DomHelper.getLocationObject((Element)nl.item(0)); - + + Location loc = DomHelper.getLocationObject((Element) nl.item(0)); + assertNotNull(loc); - assertTrue("Should be line 6, was "+loc.getLineNumber(), - 6==loc.getLineNumber()); + assertEquals("Should be line 3, was " + loc.getLineNumber(), 3, loc.getLineNumber()); + } + + public void testExternalEntities() { + String dtdFile = getClass().getResource("/author.dtd").getPath(); + String xml = "<!DOCTYPE foo [<!ELEMENT foo (bar)><!ELEMENT bar (#PCDATA)><!ENTITY writer SYSTEM \"file://" + dtdFile + "\">]><foo><bar>&writer;</bar></foo>"; + InputSource in = new InputSource(new StringReader(xml)); + in.setSystemId("foo://bar"); + + Document doc = DomHelper.parse(in); + assertNotNull(doc); + assertEquals("Wrong root node", "foo", doc.getDocumentElement().getNodeName()); + + NodeList nl = doc.getElementsByTagName("bar"); + assertEquals(1, nl.getLength()); + assertNull(nl.item(0).getNodeValue()); } }
