From 05c84ff8304a69a30b251f207a7b93c2c882564d Mon Sep 17 00:00:00 2001 From: Mark Emlyn David Thomas Date: Mon, 9 Dec 2013 10:01:16 +0000 Subject: [PATCH] Add an option to the Context to control the blocking of XML external entities when parsing XML configuration files and enable this blocking by default. The block is implemented via a custom resolver to enable the logging of any blocked entities. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1549528 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/Context.java | 19 +++ java/org/apache/catalina/Globals.java | 11 ++ .../apache/catalina/ant/ValidatorTask.java | 6 +- .../catalina/core/ApplicationContext.java | 17 ++- .../apache/catalina/core/StandardContext.java | 19 +++ .../catalina/startup/ContextConfig.java | 2 +- .../catalina/startup/FailedContext.java | 5 + java/org/apache/jasper/Constants.java | 9 ++ java/org/apache/jasper/JspC.java | 18 ++- .../compiler/ImplicitTagLibraryInfo.java | 15 ++- .../jasper/compiler/JspDocumentParser.java | 42 ++++++- .../jasper/compiler/TagPluginManager.java | 12 +- java/org/apache/jasper/compiler/TldCache.java | 10 +- .../jasper/servlet/JasperInitializer.java | 10 +- .../jasper/servlet/JspCServletContext.java | 16 ++- .../org/apache/jasper/servlet/TldScanner.java | 6 +- .../util/descriptor/DigesterFactory.java | 23 +++- .../tomcat/util/descriptor/LocalResolver.java | 118 +++++++++++------- .../util/descriptor/LocalStrings.properties | 2 + .../descriptor/tagplugin/TagPluginParser.java | 5 +- .../tomcat/util/descriptor/tld/TldParser.java | 11 +- .../util/descriptor/web/WebXmlParser.java | 7 +- .../resources/TestSchemaValidation.java | 28 ++--- .../apache/catalina/core/TesterContext.java | 10 ++ .../apache/jasper/servlet/TestTldScanner.java | 3 +- .../util/descriptor/TestLocalResolver.java | 21 ++-- .../util/descriptor/tld/TestTldParser.java | 2 +- webapps/docs/config/context.xml | 10 ++ webapps/docs/security-howto.xml | 3 + 29 files changed, 351 insertions(+), 109 deletions(-) diff --git a/java/org/apache/catalina/Context.java b/java/org/apache/catalina/Context.java index dab578fbe0e0..da17ac027e26 100644 --- a/java/org/apache/catalina/Context.java +++ b/java/org/apache/catalina/Context.java @@ -616,6 +616,25 @@ public void setSessionCookiePathUsesTrailingSlash( public void setXmlValidation(boolean xmlValidation); + /** + * Will the parsing of web.xml, web-fragment.xml, *.tld, *.jspx, *.tagx and + * tagplugin.xml files for this Context block the use of external entities? + * + * @return true if access to external entities is blocked + */ + public boolean getXmlBlockExternal(); + + + /** + * Controls whether the parsing of web.xml, web-fragment.xml, *.tld, *.jspx, + * *.tagx and tagplugin.xml files for this Context will block the use of + * external entities. + * + * @param xmlBlockExternal true to block external entities + */ + public void setXmlBlockExternal(boolean xmlBlockExternal); + + /** * Will the parsing of *.tld files for this Context be performed by a * validating parser? diff --git a/java/org/apache/catalina/Globals.java b/java/org/apache/catalina/Globals.java index 9cda32b22e10..7705cf73d0ed 100644 --- a/java/org/apache/catalina/Globals.java +++ b/java/org/apache/catalina/Globals.java @@ -279,4 +279,15 @@ public final class Globals { */ public static final String JASPER_XML_VALIDATION_TLD_INIT_PARAM = "org.apache.jasper.XML_VALIDATE_TLD"; + + + /** + * Name of the ServletContext init-param that determines if the JSP engine + * will block external entities from being used in *.tld, *.jspx, *.tagx and + * tagplugin.xml files. + *

+ * This must be kept in sync with org.apache.jasper.Constants + */ + public static final String JASPER_XML_BLOCK_EXTERNAL_INIT_PARAM = + "org.apache.jasper.XML_BLOCK_EXTERNAL"; } diff --git a/java/org/apache/catalina/ant/ValidatorTask.java b/java/org/apache/catalina/ant/ValidatorTask.java index 935c1aabda3e..4544b08aa3be 100644 --- a/java/org/apache/catalina/ant/ValidatorTask.java +++ b/java/org/apache/catalina/ant/ValidatorTask.java @@ -24,6 +24,7 @@ import java.io.FileInputStream; import java.io.InputStream; +import org.apache.catalina.Globals; import org.apache.catalina.startup.Constants; import org.apache.tomcat.util.descriptor.DigesterFactory; import org.apache.tomcat.util.digester.Digester; @@ -90,7 +91,10 @@ public void execute() throws BuildException { Thread.currentThread().setContextClassLoader (ValidatorTask.class.getClassLoader()); - Digester digester = DigesterFactory.newDigester(true, true, null); + // Called through trusted manager interface. If running under a + // SecurityManager assume that untrusted applications may be deployed. + Digester digester = DigesterFactory.newDigester( + true, true, null, Globals.IS_SECURITY_ENABLED); try { file = file.getCanonicalFile(); InputStream stream = diff --git a/java/org/apache/catalina/core/ApplicationContext.java b/java/org/apache/catalina/core/ApplicationContext.java index 1a32ff8c513e..8a48d332388d 100644 --- a/java/org/apache/catalina/core/ApplicationContext.java +++ b/java/org/apache/catalina/core/ApplicationContext.java @@ -304,12 +304,20 @@ public String getContextPath() { */ @Override public String getInitParameter(final String name) { - // Special handling for XML validation as the context setting must + // Special handling for XML settings as the context setting must // always override anything that might have been set by an application. if (Globals.JASPER_XML_VALIDATION_TLD_INIT_PARAM.equals(name) && context.getTldValidation()) { return "true"; } + if (Globals.JASPER_XML_BLOCK_EXTERNAL_INIT_PARAM.equals(name)) { + if (context.getXmlBlockExternal()) { + return "true"; + } else if (Globals.IS_SECURITY_ENABLED) { + // System admin has explicitly changed the default + return "false"; + } + } return parameters.get(name); } @@ -322,11 +330,14 @@ public String getInitParameter(final String name) { public Enumeration getInitParameterNames() { Set names = new HashSet<>(); names.addAll(parameters.keySet()); - // Special handling for XML validation as this attribute will always be - // available if validation has been enabled on the context + // Special handling for XML settings as these attributes will always be + // available if they have been set on the context if (context.getTldValidation()) { names.add(Globals.JASPER_XML_VALIDATION_TLD_INIT_PARAM); } + if (context.getXmlBlockExternal() || Globals.IS_SECURITY_ENABLED) { + names.add(Globals.JASPER_XML_BLOCK_EXTERNAL_INIT_PARAM); + } return Collections.enumeration(names); } diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index 1eee6b261689..71b0d6476b64 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -699,6 +699,13 @@ public StandardContext() { */ private boolean webXmlNamespaceAware = Globals.STRICT_SERVLET_COMPLIANCE; + + /** + * Attribute used to turn on/off the use of external entities. + */ + private boolean xmlBlockExternal = Globals.IS_SECURITY_ENABLED; + + /** * Attribute value used to turn on/off XML validation */ @@ -6386,6 +6393,18 @@ public boolean getXmlValidation() { } + @Override + public void setXmlBlockExternal(boolean xmlBlockExternal) { + this.xmlBlockExternal = xmlBlockExternal; + } + + + @Override + public boolean getXmlBlockExternal() { + return xmlBlockExternal; + } + + @Override public void setTldValidation(boolean tldValidation) { this.tldValidation = tldValidation; diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java index b4cf9359077e..0303955297b0 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -730,7 +730,7 @@ protected void init() { contextConfig(contextDigester); webXmlParser = new WebXmlParser(context.getXmlNamespaceAware(), - context.getXmlValidation()); + context.getXmlValidation(), context.getXmlBlockExternal()); } diff --git a/java/org/apache/catalina/startup/FailedContext.java b/java/org/apache/catalina/startup/FailedContext.java index c995c43a8ed8..7b04f3f66b70 100644 --- a/java/org/apache/catalina/startup/FailedContext.java +++ b/java/org/apache/catalina/startup/FailedContext.java @@ -455,6 +455,11 @@ public void setXmlNamespaceAware(boolean xmlNamespaceAware) { /* NO-OP */ } @Override public void setXmlValidation(boolean xmlValidation) { /* NO-OP */ } + @Override + public boolean getXmlBlockExternal() { return true; } + @Override + public void setXmlBlockExternal(boolean xmlBlockExternal) { /* NO-OP */ } + @Override public boolean getTldValidation() { return false; } @Override diff --git a/java/org/apache/jasper/Constants.java b/java/org/apache/jasper/Constants.java index aa39683de41f..d8022014597c 100644 --- a/java/org/apache/jasper/Constants.java +++ b/java/org/apache/jasper/Constants.java @@ -160,4 +160,13 @@ public class Constants { */ public static final String XML_VALIDATION_TLD_INIT_PARAM = "org.apache.jasper.XML_VALIDATE_TLD"; + + /** + * Name of the ServletContext init-param that determines if the XML parsers + * will block the resolution of external entities. + *

+ * This must be kept in sync with org.apache.catalina.Globals + */ + public static final String XML_BLOCK_EXTERNAL_INIT_PARAM = + "org.apache.jasper.XML_BLOCK_EXTERNAL"; } diff --git a/java/org/apache/jasper/JspC.java b/java/org/apache/jasper/JspC.java index f53a27b16535..9b7b53de112a 100644 --- a/java/org/apache/jasper/JspC.java +++ b/java/org/apache/jasper/JspC.java @@ -134,6 +134,7 @@ public class JspC extends Task implements Options { protected static final String SWITCH_SMAP = "-smap"; protected static final String SWITCH_DUMP_SMAP = "-dumpsmap"; protected static final String SWITCH_VALIDATE_TLD = "-validateTld"; + protected static final String SWITCH_BLOCK_EXTERNAL = "-blockExternal"; protected static final String SHOW_SUCCESS ="-s"; protected static final String LIST_ERRORS = "-l"; protected static final int INC_WEBXML = 10; @@ -165,6 +166,7 @@ public class JspC extends Task implements Options { protected boolean trimSpaces = false; protected boolean genStringAsCharArray = false; protected boolean validateTld; + protected boolean blockExternal; protected boolean xpoweredBy; protected boolean mappedFile = false; protected boolean poolingEnabled = true; @@ -373,6 +375,8 @@ public void setArgs(String[] arg) throws JasperException { smapDumped = true; } else if (tok.equals(SWITCH_VALIDATE_TLD)) { setValidateTld(true); + } else if (tok.equals(SWITCH_BLOCK_EXTERNAL)) { + setBlockExternal(true); } else { if (tok.startsWith("-")) { throw new JasperException("Unrecognized option: " + tok + @@ -860,6 +864,14 @@ public boolean isValidateTld() { return validateTld; } + public void setBlockExternal( boolean b ) { + this.blockExternal = b; + } + + public boolean isBlockExternal() { + return blockExternal; + } + public void setListErrors( boolean b ) { listErrors = b; } @@ -1440,8 +1452,12 @@ protected void initServletContext(ClassLoader classLoader) if (isValidateTld()) { context.setInitParameter(Constants.XML_VALIDATION_TLD_INIT_PARAM, "true"); } + if (isBlockExternal()) { + context.setInitParameter(Constants.XML_BLOCK_EXTERNAL_INIT_PARAM, "true"); + } - TldScanner scanner = new TldScanner(context, true, isValidateTld()); + TldScanner scanner = new TldScanner( + context, true, isValidateTld(), isBlockExternal()); scanner.setClassLoader(classLoader); try { diff --git a/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java b/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java index f7996fc7e7af..28e28947136e 100644 --- a/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java +++ b/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java @@ -24,6 +24,7 @@ import java.util.Set; import java.util.Vector; +import javax.servlet.ServletContext; import javax.servlet.jsp.tagext.FunctionInfo; import javax.servlet.jsp.tagext.TagFileInfo; import javax.servlet.jsp.tagext.TagInfo; @@ -119,10 +120,20 @@ public ImplicitTagLibraryInfo(JspCompilationContext ctxt, try { URL url = ctxt.getResource(path); TldResourcePath resourcePath = new TldResourcePath(url, path); + ServletContext servletContext = ctxt.getServletContext(); boolean validate = Boolean.parseBoolean( - ctxt.getServletContext().getInitParameter( + servletContext.getInitParameter( Constants.XML_VALIDATION_TLD_INIT_PARAM)); - TldParser parser = new TldParser(true, validate, new ImplicitTldRuleSet()); + String blockExternalString = servletContext.getInitParameter( + Constants.XML_BLOCK_EXTERNAL_INIT_PARAM); + boolean blockExternal; + if (blockExternalString == null) { + blockExternal = Constants.IS_SECURITY_ENABLED; + } else { + blockExternal = Boolean.parseBoolean(blockExternalString); + } + TldParser parser = new TldParser(true, validate, + new ImplicitTldRuleSet(), blockExternal); taglibXml = parser.parse(resourcePath); } catch (IOException | SAXException e) { err.jspError(e); diff --git a/java/org/apache/jasper/compiler/JspDocumentParser.java b/java/org/apache/jasper/compiler/JspDocumentParser.java index 286749d33cda..fca5905caaea 100644 --- a/java/org/apache/jasper/compiler/JspDocumentParser.java +++ b/java/org/apache/jasper/compiler/JspDocumentParser.java @@ -28,8 +28,11 @@ import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; +import org.apache.jasper.Constants; import org.apache.jasper.JasperException; import org.apache.jasper.JspCompilationContext; +import org.apache.tomcat.util.descriptor.DigesterFactory; +import org.apache.tomcat.util.descriptor.LocalResolver; import org.apache.tomcat.util.descriptor.tld.TldResourcePath; import org.apache.tomcat.util.scan.Jar; import org.xml.sax.Attributes; @@ -39,6 +42,7 @@ import org.xml.sax.SAXParseException; import org.xml.sax.XMLReader; import org.xml.sax.ext.DefaultHandler2; +import org.xml.sax.ext.EntityResolver2; import org.xml.sax.helpers.AttributesImpl; /** @@ -91,6 +95,7 @@ class JspDocumentParser private boolean inDTD; private boolean isValidating; + private final EntityResolver2 entityResolver; private final ErrorDispatcher err; private final boolean isTagFile; @@ -119,6 +124,20 @@ public JspDocumentParser( this.isTagFile = isTagFile; this.directivesOnly = directivesOnly; this.isTop = true; + + String blockExternalString = ctxt.getServletContext().getInitParameter( + Constants.XML_BLOCK_EXTERNAL_INIT_PARAM); + boolean blockExternal; + if (blockExternalString == null) { + blockExternal = Constants.IS_SECURITY_ENABLED; + } else { + blockExternal = Boolean.parseBoolean(blockExternalString); + } + + this.entityResolver = new LocalResolver( + DigesterFactory.SERVLET_API_PUBLIC_IDS, + DigesterFactory.SERVLET_API_SYSTEM_IDS, + blockExternal); } /* @@ -232,13 +251,26 @@ private void addInclude(Node parent, Collection files) throws SAXExcepti } } + + @Override + public InputSource getExternalSubset(String name, String baseURI) + throws SAXException, IOException { + return entityResolver.getExternalSubset(name, baseURI); + } + @Override - public InputSource resolveEntity(String name, String publicId, String baseURI, String systemId) + public InputSource resolveEntity(String publicId, String systemId) throws SAXException, IOException { - // TODO URLs returned by the Jar abstraction may be of the form jar:jar: which - // is not a URL that can be resolved by the JRE. This should use the JarFactory - // to construct and return a valid InputSource. - return null; + return entityResolver.resolveEntity(publicId, systemId); + } + + @Override + public InputSource resolveEntity(String name, String publicId, + String baseURI, String systemId) throws SAXException, IOException { + // TODO URLs returned by the Jar abstraction may be of the form jar:jar: + // which is not a URL that can be resolved by the JRE. This should + // use the JarFactory to construct and return a valid InputSource. + return entityResolver.resolveEntity(name, publicId, baseURI, systemId); } /* diff --git a/java/org/apache/jasper/compiler/TagPluginManager.java b/java/org/apache/jasper/compiler/TagPluginManager.java index af9d536f0b55..05e9b901385f 100644 --- a/java/org/apache/jasper/compiler/TagPluginManager.java +++ b/java/org/apache/jasper/compiler/TagPluginManager.java @@ -24,6 +24,7 @@ import javax.servlet.ServletContext; +import org.apache.jasper.Constants; import org.apache.jasper.JasperException; import org.apache.jasper.compiler.tagplugin.TagPlugin; import org.apache.jasper.compiler.tagplugin.TagPluginContext; @@ -61,7 +62,16 @@ private void init(ErrorDispatcher err) throws JasperException { if (initialized) return; - TagPluginParser parser = new TagPluginParser(ctxt); + String blockExternalString = ctxt.getInitParameter( + Constants.XML_BLOCK_EXTERNAL_INIT_PARAM); + boolean blockExternal; + if (blockExternalString == null) { + blockExternal = Constants.IS_SECURITY_ENABLED; + } else { + blockExternal = Boolean.parseBoolean(blockExternalString); + } + + TagPluginParser parser = new TagPluginParser(ctxt, blockExternal); try { Enumeration urls = diff --git a/java/org/apache/jasper/compiler/TldCache.java b/java/org/apache/jasper/compiler/TldCache.java index cf2012cde8de..6ba105a8dc4b 100644 --- a/java/org/apache/jasper/compiler/TldCache.java +++ b/java/org/apache/jasper/compiler/TldCache.java @@ -74,7 +74,15 @@ public TldCache(ServletContext servletContext, } boolean validate = Boolean.parseBoolean( servletContext.getInitParameter(Constants.XML_VALIDATION_TLD_INIT_PARAM)); - tldParser = new TldParser(true, validate); + String blockExternalString = servletContext.getInitParameter( + Constants.XML_BLOCK_EXTERNAL_INIT_PARAM); + boolean blockExternal; + if (blockExternalString == null) { + blockExternal = Constants.IS_SECURITY_ENABLED; + } else { + blockExternal = Boolean.parseBoolean(blockExternalString); + } + tldParser = new TldParser(true, validate, blockExternal); } diff --git a/java/org/apache/jasper/servlet/JasperInitializer.java b/java/org/apache/jasper/servlet/JasperInitializer.java index 9eb8e68fbeb1..0a399cf0862f 100644 --- a/java/org/apache/jasper/servlet/JasperInitializer.java +++ b/java/org/apache/jasper/servlet/JasperInitializer.java @@ -80,9 +80,17 @@ public void onStartup(Set> types, ServletContext context) throws Servle boolean validate = Boolean.parseBoolean( context.getInitParameter(Constants.XML_VALIDATION_TLD_INIT_PARAM)); + String blockExternalString = context.getInitParameter( + Constants.XML_BLOCK_EXTERNAL_INIT_PARAM); + boolean blockExternal; + if (blockExternalString == null) { + blockExternal = Constants.IS_SECURITY_ENABLED; + } else { + blockExternal = Boolean.parseBoolean(blockExternalString); + } // scan the application for TLDs - TldScanner scanner = new TldScanner(context, true, validate); + TldScanner scanner = new TldScanner(context, true, validate, blockExternal); try { scanner.scan(); } catch (IOException | SAXException e) { diff --git a/java/org/apache/jasper/servlet/JspCServletContext.java b/java/org/apache/jasper/servlet/JspCServletContext.java index 5071c83d8fb6..34991c697d65 100644 --- a/java/org/apache/jasper/servlet/JspCServletContext.java +++ b/java/org/apache/jasper/servlet/JspCServletContext.java @@ -44,11 +44,11 @@ import javax.servlet.SessionTrackingMode; import javax.servlet.descriptor.JspConfigDescriptor; +import org.apache.jasper.Constants; import org.apache.jasper.JasperException; import org.apache.jasper.compiler.Localizer; import org.apache.jasper.util.ExceptionUtils; import org.apache.tomcat.JarScanType; -import org.apache.tomcat.util.descriptor.web.Constants; import org.apache.tomcat.util.descriptor.web.FragmentJarScannerCallback; import org.apache.tomcat.util.descriptor.web.WebXml; import org.apache.tomcat.util.descriptor.web.WebXmlParser; @@ -124,13 +124,21 @@ public JspCServletContext(PrintWriter aLogWriter, URL aResourceBaseURL, ClassLoa private WebXml buildMergedWebXml() throws JasperException { WebXml webXml = new WebXml(); - - WebXmlParser webXmlParser = new WebXmlParser(false, false); + String blockExternalString = getInitParameter( + Constants.XML_BLOCK_EXTERNAL_INIT_PARAM); + boolean blockExternal; + if (blockExternalString == null) { + blockExternal = Constants.IS_SECURITY_ENABLED; + } else { + blockExternal = Boolean.parseBoolean(blockExternalString); + } + WebXmlParser webXmlParser = new WebXmlParser(false, false, blockExternal); // Use this class's classloader as Ant will have set the TCCL to its own webXmlParser.setClassLoader(getClass().getClassLoader()); try { - URL url = getResource(Constants.WEB_XML_LOCATION); + URL url = getResource( + org.apache.tomcat.util.descriptor.web.Constants.WEB_XML_LOCATION); if (!webXmlParser.parseWebXml(url, webXml, false)) { throw new JasperException(Localizer.getMessage("jspc.error.invalidWebXml")); } diff --git a/java/org/apache/jasper/servlet/TldScanner.java b/java/org/apache/jasper/servlet/TldScanner.java index e8977b29215d..94f995afc8a0 100644 --- a/java/org/apache/jasper/servlet/TldScanner.java +++ b/java/org/apache/jasper/servlet/TldScanner.java @@ -72,9 +72,11 @@ public class TldScanner { */ public TldScanner(ServletContext context, boolean namespaceAware, - boolean validation) { + boolean validation, + boolean blockExternal) { this.context = context; - this.tldParser = new TldParser(namespaceAware, validation); + + this.tldParser = new TldParser(namespaceAware, validation, blockExternal); } /** diff --git a/java/org/apache/tomcat/util/descriptor/DigesterFactory.java b/java/org/apache/tomcat/util/descriptor/DigesterFactory.java index 100a6c6428a3..57a2940e2865 100644 --- a/java/org/apache/tomcat/util/descriptor/DigesterFactory.java +++ b/java/org/apache/tomcat/util/descriptor/DigesterFactory.java @@ -16,6 +16,7 @@ */ package org.apache.tomcat.util.descriptor; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -23,6 +24,7 @@ import org.apache.tomcat.util.digester.Digester; import org.apache.tomcat.util.digester.RuleSet; +import org.xml.sax.ext.EntityResolver2; /** * Wrapper class around the Digester that hide Digester's initialization @@ -31,10 +33,16 @@ public class DigesterFactory { /** - * A resolver for the resources packaged in servlet-api.jar + * Mapping of well-known public IDs used by the Servlet API to the matching + * local resource. */ - public static final LocalResolver SERVLET_API_RESOLVER; + public static final Map SERVLET_API_PUBLIC_IDS; + /** + * Mapping of well-known system IDs used by the Servlet API to the matching + * local resource. + */ + public static final Map SERVLET_API_SYSTEM_IDS; static { Map publicIds = new HashMap<>(); @@ -89,7 +97,8 @@ public class DigesterFactory { addSelf(systemIds, "javaee_web_services_1_4.xsd"); addSelf(systemIds, "javaee_web_services_client_1_4.xsd"); - SERVLET_API_RESOLVER = new LocalResolver(publicIds, systemIds); + SERVLET_API_PUBLIC_IDS = Collections.unmodifiableMap(publicIds); + SERVLET_API_SYSTEM_IDS = Collections.unmodifiableMap(systemIds); } private static void addSelf(Map ids, String id) { @@ -107,15 +116,19 @@ private static String idFor(String url) { * @param xmlValidation turn on/off xml validation * @param xmlNamespaceAware turn on/off namespace validation * @param rule an instance of RuleSet used for parsing the xml. + * @param blockExternal turn on/off the blocking of external resources */ public static Digester newDigester(boolean xmlValidation, boolean xmlNamespaceAware, - RuleSet rule) { + RuleSet rule, + boolean blockExternal) { Digester digester = new Digester(); digester.setNamespaceAware(xmlNamespaceAware); digester.setValidating(xmlValidation); digester.setUseContextClassLoader(true); - digester.setEntityResolver(SERVLET_API_RESOLVER); + EntityResolver2 resolver = new LocalResolver(SERVLET_API_PUBLIC_IDS, + SERVLET_API_SYSTEM_IDS, blockExternal); + digester.setEntityResolver(resolver); if (rule != null) { digester.addRuleSet(rule); } diff --git a/java/org/apache/tomcat/util/descriptor/LocalResolver.java b/java/org/apache/tomcat/util/descriptor/LocalResolver.java index 55edf3d834ac..fb191459690d 100644 --- a/java/org/apache/tomcat/util/descriptor/LocalResolver.java +++ b/java/org/apache/tomcat/util/descriptor/LocalResolver.java @@ -16,6 +16,7 @@ */ package org.apache.tomcat.util.descriptor; +import java.io.FileNotFoundException; import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; @@ -23,6 +24,7 @@ import java.net.URL; import java.util.Map; +import org.apache.tomcat.util.res.StringManager; import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.ext.EntityResolver2; @@ -32,22 +34,30 @@ */ public class LocalResolver implements EntityResolver2 { + private static final StringManager sm = + StringManager.getManager(Constants.PACKAGE_NAME); + private final Map publicIds; private final Map systemIds; - + private final boolean blockExternal; /** * Constructor providing mappings of public and system identifiers to local * resources. Each map contains a mapping from a well-known identifier to a * URL for a local resource path. * - * @param publicIds mapping of public identifiers to local resources - * @param systemIds mapping of system identifiers to local resources + * @param publicIds mapping of well-known public identifiers to local + * resources + * @param systemIds mapping of well-known system identifiers to local + * resources + * @param blockExternal are external resources blocked that are not + * well-known */ public LocalResolver(Map publicIds, - Map systemIds) { + Map systemIds, boolean blockExternal) { this.publicIds = publicIds; this.systemIds = systemIds; + this.blockExternal = blockExternal; } @@ -60,63 +70,77 @@ public InputSource resolveEntity(String publicId, String systemId) @Override public InputSource resolveEntity(String name, String publicId, - String baseURI, String systemId) throws SAXException, IOException { - - String resolved = resolve(publicId, systemId, baseURI); - if (resolved == null) { - return null; - } + String base, String systemId) throws SAXException, IOException { - InputSource is = new InputSource(resolved); - is.setPublicId(publicId); - return is; - } - - - @Override - public InputSource getExternalSubset(String name, String baseURI) - throws SAXException, IOException { - return null; - } - - - private String resolve(String publicId, String systemId, String baseURI) { - // try resolving using the publicId + // First try resolving using the publicId String resolved = publicIds.get(publicId); if (resolved != null) { - return resolved; + InputSource is = new InputSource(resolved); + is.setPublicId(publicId); + return is; } - // try resolving using the systemId + // If there is no systemId, can't try anything else if (systemId == null) { - return null; + throw new FileNotFoundException(sm.getString("localResolver.unresolvedEntity", + name, publicId, systemId, base)); } - systemId = resolve(baseURI, systemId); + // Try resolving with the supplied systemId resolved = systemIds.get(systemId); if (resolved != null) { - return resolved; + InputSource is = new InputSource(resolved); + is.setPublicId(publicId); + return is; } - // fall back to the supplied systemId - return systemId; - } - - - private static String resolve(String baseURI, String systemId) { + // Resolve the supplied systemId against the base + URI systemUri; try { - if (baseURI == null) { - return systemId; + if (base == null) { + systemUri = new URI(systemId); + } else { + // Can't use URI.resolve() because "jar:..." URLs are not valid + // hierarchical URIs so resolve() does not work. new URL() + // delegates to the jar: stream handler and it manages to figure + // it out. + URI baseUri = new URI(base); + systemUri = new URL(baseUri.toURL(), systemId).toURI(); } - URI systemUri = new URI(systemId); - if (systemUri.isAbsolute()) { - return systemId; - } - return new URL(new URL(baseURI), systemId).toString(); + systemUri = systemUri.normalize(); } catch (URISyntaxException e) { - return systemId; - } catch (MalformedURLException e) { - return systemId; + // May be caused by a | being used instead of a : in an absolute + // file URI on Windows. + if (blockExternal) { + // Absolute paths aren't allowed so block it + throw new MalformedURLException(e.getMessage()); + } else { + // See if the URLHandler can resolve it + return new InputSource(systemId); + } + } + if (systemUri.isAbsolute()) { + // Try the resolved systemId + resolved = systemIds.get(systemUri.toString()); + if (resolved != null) { + InputSource is = new InputSource(resolved); + is.setPublicId(publicId); + return is; + } + if (!blockExternal) { + InputSource is = new InputSource(systemUri.toString()); + is.setPublicId(publicId); + return is; + } } + throw new FileNotFoundException(sm.getString("localResolver.unresolvedEntity", + name, publicId, systemId, base)); + } + + + @Override + public InputSource getExternalSubset(String name, String baseURI) + throws SAXException, IOException { + return null; } -} \ No newline at end of file +} diff --git a/java/org/apache/tomcat/util/descriptor/LocalStrings.properties b/java/org/apache/tomcat/util/descriptor/LocalStrings.properties index f7d68915ad01..2e42fc719368 100644 --- a/java/org/apache/tomcat/util/descriptor/LocalStrings.properties +++ b/java/org/apache/tomcat/util/descriptor/LocalStrings.properties @@ -13,5 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +localResolver.unresolvedEntity=Could not resolve XML resource [{0}] with public ID [{1}], system ID [{2}] and base URI [{3}] to a known, local entity. + xmlErrorHandler.error=Non-fatal error [{0}] reported processing [{1}]. xmlErrorHandler.warning=Warning [{0}] reported processing [{1}]. diff --git a/java/org/apache/tomcat/util/descriptor/tagplugin/TagPluginParser.java b/java/org/apache/tomcat/util/descriptor/tagplugin/TagPluginParser.java index 56bad88b5c1e..b3be866b1eee 100644 --- a/java/org/apache/tomcat/util/descriptor/tagplugin/TagPluginParser.java +++ b/java/org/apache/tomcat/util/descriptor/tagplugin/TagPluginParser.java @@ -42,8 +42,9 @@ public class TagPluginParser { private final Digester digester; private final Map plugins = new HashMap<>(); - public TagPluginParser(ServletContext context) { - digester = DigesterFactory.newDigester(false, false, new TagPluginRuleSet()); + public TagPluginParser(ServletContext context, boolean blockExternal) { + digester = DigesterFactory.newDigester( + false, false, new TagPluginRuleSet(), blockExternal); digester.setClassLoader(context.getClassLoader()); } diff --git a/java/org/apache/tomcat/util/descriptor/tld/TldParser.java b/java/org/apache/tomcat/util/descriptor/tld/TldParser.java index 4bbb0b95bc72..9166d8c119ea 100644 --- a/java/org/apache/tomcat/util/descriptor/tld/TldParser.java +++ b/java/org/apache/tomcat/util/descriptor/tld/TldParser.java @@ -35,12 +35,15 @@ public class TldParser { private static final Log log = LogFactory.getLog(TldParser.class); private final Digester digester; - public TldParser(boolean namespaceAware, boolean validation) { - this(namespaceAware, validation, new TldRuleSet()); + public TldParser(boolean namespaceAware, boolean validation, + boolean blockExternal) { + this(namespaceAware, validation, new TldRuleSet(), blockExternal); } - public TldParser(boolean namespaceAware, boolean validation, RuleSet ruleSet) { - digester = DigesterFactory.newDigester(validation, namespaceAware, ruleSet); + public TldParser(boolean namespaceAware, boolean validation, RuleSet ruleSet, + boolean blockExternal) { + digester = DigesterFactory.newDigester( + validation, namespaceAware, ruleSet, blockExternal); } public TaglibXml parse(TldResourcePath path) throws IOException, SAXException { diff --git a/java/org/apache/tomcat/util/descriptor/web/WebXmlParser.java b/java/org/apache/tomcat/util/descriptor/web/WebXmlParser.java index 8b9ff3b2fea5..6b40d2e8a390 100644 --- a/java/org/apache/tomcat/util/descriptor/web/WebXmlParser.java +++ b/java/org/apache/tomcat/util/descriptor/web/WebXmlParser.java @@ -55,15 +55,16 @@ public class WebXmlParser { private final WebRuleSet webFragmentRuleSet; - public WebXmlParser(boolean namespaceAware, boolean validation) { + public WebXmlParser(boolean namespaceAware, boolean validation, + boolean blockExternal) { webRuleSet = new WebRuleSet(false); webDigester = DigesterFactory.newDigester(validation, - namespaceAware, webRuleSet); + namespaceAware, webRuleSet, blockExternal); webDigester.getParser(); webFragmentRuleSet = new WebRuleSet(true); webFragmentDigester = DigesterFactory.newDigester(validation, - namespaceAware, webFragmentRuleSet); + namespaceAware, webFragmentRuleSet, blockExternal); webFragmentDigester.getParser(); } diff --git a/test/javax/servlet/resources/TestSchemaValidation.java b/test/javax/servlet/resources/TestSchemaValidation.java index 5715154c5c55..1a6149ba1a63 100644 --- a/test/javax/servlet/resources/TestSchemaValidation.java +++ b/test/javax/servlet/resources/TestSchemaValidation.java @@ -31,8 +31,8 @@ public class TestSchemaValidation { @Test public void testWebapp() throws Exception { - Digester digester = - DigesterFactory.newDigester(true, true, new WebRuleSet(false)); + Digester digester = DigesterFactory.newDigester( + true, true, new WebRuleSet(false), true); digester.push(new WebXml()); WebXml desc = (WebXml) digester.parse( new File("test/webapp/WEB-INF/web.xml")); @@ -41,8 +41,8 @@ public void testWebapp() throws Exception { @Test public void testWebapp_2_2() throws Exception { - Digester digester = - DigesterFactory.newDigester(true, true, new WebRuleSet(false)); + Digester digester = DigesterFactory.newDigester( + true, true, new WebRuleSet(false), true); digester.push(new WebXml()); WebXml desc = (WebXml) digester.parse( new File("test/webapp-2.2/WEB-INF/web.xml")); @@ -52,8 +52,8 @@ public void testWebapp_2_2() throws Exception { @Test public void testWebapp_2_3() throws Exception { - Digester digester = - DigesterFactory.newDigester(true, true, new WebRuleSet(false)); + Digester digester = DigesterFactory.newDigester( + true, true, new WebRuleSet(false), true); digester.push(new WebXml()); WebXml desc = (WebXml) digester.parse( new File("test/webapp-2.3/WEB-INF/web.xml")); @@ -63,8 +63,8 @@ public void testWebapp_2_3() throws Exception { @Test public void testWebapp_2_4() throws Exception { - Digester digester = - DigesterFactory.newDigester(true, true, new WebRuleSet(false)); + Digester digester = DigesterFactory.newDigester( + true, true, new WebRuleSet(false), true); digester.push(new WebXml()); WebXml desc = (WebXml) digester.parse( new File("test/webapp-2.4/WEB-INF/web.xml")); @@ -73,8 +73,8 @@ public void testWebapp_2_4() throws Exception { @Test public void testWebapp_2_5() throws Exception { - Digester digester = - DigesterFactory.newDigester(true, true, new WebRuleSet(false)); + Digester digester = DigesterFactory.newDigester( + true, true, new WebRuleSet(false), true); digester.push(new WebXml()); WebXml desc = (WebXml) digester.parse( new File("test/webapp-2.5/WEB-INF/web.xml")); @@ -83,8 +83,8 @@ public void testWebapp_2_5() throws Exception { @Test public void testWebapp_3_0() throws Exception { - Digester digester = - DigesterFactory.newDigester(true, true, new WebRuleSet(false)); + Digester digester = DigesterFactory.newDigester( + true, true, new WebRuleSet(false), true); digester.push(new WebXml()); WebXml desc = (WebXml) digester.parse( new File("test/webapp-3.0/WEB-INF/web.xml")); @@ -93,8 +93,8 @@ public void testWebapp_3_0() throws Exception { @Test public void testWebapp_3_1() throws Exception { - Digester digester = - DigesterFactory.newDigester(true, true, new WebRuleSet(false)); + Digester digester = DigesterFactory.newDigester( + true, true, new WebRuleSet(false), true); digester.push(new WebXml()); WebXml desc = (WebXml) digester.parse( new File("test/webapp-3.1/WEB-INF/web.xml")); diff --git a/test/org/apache/catalina/core/TesterContext.java b/test/org/apache/catalina/core/TesterContext.java index 9449fdcb700a..59801e434f91 100644 --- a/test/org/apache/catalina/core/TesterContext.java +++ b/test/org/apache/catalina/core/TesterContext.java @@ -642,6 +642,16 @@ public void setXmlValidation(boolean xmlValidation) { // NO-OP } + @Override + public boolean getXmlBlockExternal() { + return false; + } + + @Override + public void setXmlBlockExternal(boolean xmlBlockExternal) { + // NO-OP + } + @Override public boolean getTldValidation(){ return false; diff --git a/test/org/apache/jasper/servlet/TestTldScanner.java b/test/org/apache/jasper/servlet/TestTldScanner.java index 03a0e4dfa126..6e617688d1f2 100644 --- a/test/org/apache/jasper/servlet/TestTldScanner.java +++ b/test/org/apache/jasper/servlet/TestTldScanner.java @@ -39,7 +39,8 @@ public void testWithWebapp() throws Exception { Context context = tomcat.addWebapp(null, "/test", appDir.getAbsolutePath()); tomcat.start(); - TldScanner scanner = new TldScanner(context.getServletContext(), true, true); + TldScanner scanner = + new TldScanner(context.getServletContext(), true, true, true); scanner.scan(); Assert.assertEquals(5, scanner.getUriTldResourcePathMap().size()); Assert.assertEquals(1, scanner.getListeners().size()); diff --git a/test/org/apache/tomcat/util/descriptor/TestLocalResolver.java b/test/org/apache/tomcat/util/descriptor/TestLocalResolver.java index 8a1ebe156ff5..2473d16d23cd 100644 --- a/test/org/apache/tomcat/util/descriptor/TestLocalResolver.java +++ b/test/org/apache/tomcat/util/descriptor/TestLocalResolver.java @@ -16,6 +16,7 @@ */ package org.apache.tomcat.util.descriptor; +import java.io.FileNotFoundException; import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -34,7 +35,7 @@ public class TestLocalResolver { private final Map publicIds = new HashMap<>(); private final Map systemIds = new HashMap<>(); - private LocalResolver resolver = new LocalResolver(publicIds, systemIds); + private LocalResolver resolver = new LocalResolver(publicIds, systemIds, true); private String WEB_22_LOCAL; private String WEB_31_LOCAL; private String WEBCOMMON_31_LOCAL; @@ -53,25 +54,25 @@ public String urlFor(String id) { return ServletContext.class.getResource(id).toExternalForm(); } - @Test - public void unknownNullIdIsNull() throws IOException, SAXException { + @Test(expected = FileNotFoundException.class) + public void unknownNullId() throws IOException, SAXException { Assert.assertNull(resolver.resolveEntity(null, null)); } - @Test - public void unknownPublicIdIsNull() throws IOException, SAXException { + @Test(expected = FileNotFoundException.class) + public void unknownPublicId() throws IOException, SAXException { Assert.assertNull(resolver.resolveEntity("unknown", null)); } - @Test - public void unknownSystemIdIsReturned() throws IOException, SAXException { + @Test(expected = FileNotFoundException.class) + public void unknownSystemId() throws IOException, SAXException { InputSource source = resolver.resolveEntity(null, "unknown"); Assert.assertEquals(null, source.getPublicId()); Assert.assertEquals("unknown", source.getSystemId()); } - @Test - public void unknownSystemIdIsResolvedAgainstBaseURI() + @Test(expected = FileNotFoundException.class) + public void unknownRelativeSystemId() throws IOException, SAXException { InputSource source = resolver.resolveEntity( null, null, "http://example.com/home.html", "unknown"); @@ -121,4 +122,4 @@ public void absoluteSystemIdOverridesBaseURI() Assert.assertEquals(null, source.getPublicId()); Assert.assertEquals(WEB_31_LOCAL, source.getSystemId()); } -} \ No newline at end of file +} diff --git a/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java b/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java index f06eb4705005..ea70aed606a4 100644 --- a/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java +++ b/test/org/apache/tomcat/util/descriptor/tld/TestTldParser.java @@ -36,7 +36,7 @@ public class TestTldParser { @Before public void init() { - parser = new TldParser(true, true); + parser = new TldParser(true, true, null, true); } @Test diff --git a/webapps/docs/config/context.xml b/webapps/docs/config/context.xml index 7f6173ef48fe..beeca5a6cbc3 100644 --- a/webapps/docs/config/context.xml +++ b/webapps/docs/config/context.xml @@ -524,6 +524,16 @@ Context. If not specified, a standard default value will be used.

+ +

If the value of this flag is true, the parsing of + web.xml, web-fragment.xml, *.tld, + *.jspx, *.tagx and tagPlugins.xml + files for this web application will not permit external entities to be + loaded. If a SecurityManager is configured then the default + value of this attribute will be true, else the default + value will be false.

+ +

If the value of this flag is true, the parsing of web.xml and web-fragment.xml files for this diff --git a/webapps/docs/security-howto.xml b/webapps/docs/security-howto.xml index 2e4330787d28..e7180ab0cc96 100644 --- a/webapps/docs/security-howto.xml +++ b/webapps/docs/security-howto.xml @@ -179,6 +179,9 @@