Return-Path: 
 <dev-return-148187-apmail-tomcat-dev-archive=tomcat.apache.org@tomcat.apache.org>
X-Original-To: apmail-tomcat-dev-archive@www.apache.org
Delivered-To: apmail-tomcat-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 6CF1F10D08
	for <apmail-tomcat-dev-archive@www.apache.org>;
 Mon, 17 Mar 2014 21:45:17 +0000 (UTC)
Received: (qmail 65854 invoked by uid 500); 17 Mar 2014 21:45:15 -0000
Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org
Received: (qmail 65755 invoked by uid 500); 17 Mar 2014 21:45:15 -0000
Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@tomcat.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@tomcat.apache.org>
List-Post: <mailto:dev@tomcat.apache.org>
List-Id: <dev.tomcat.apache.org>
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Delivered-To: mailing list dev@tomcat.apache.org
Received: (qmail 65745 invoked by uid 99); 17 Mar 2014 21:45:15 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 17 Mar 2014 21:45:15 +0000
X-ASF-Spam-Status: No, hits=-2000.0 required=5.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 17 Mar 2014 21:45:11 +0000
Received: from eris.apache.org (localhost [127.0.0.1])
	by eris.apache.org (Postfix) with ESMTP id 743A823889CB
	for <dev@tomcat.apache.org>; Mon, 17 Mar 2014 21:44:49 +0000 (UTC)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1578611 - in /tomcat/trunk: conf/web.xml
 java/org/apache/catalina/servlets/DefaultServlet.java
 java/org/apache/catalina/servlets/LocalStrings.properties
 webapps/docs/default-servlet.xml
Date: Mon, 17 Mar 2014 21:44:49 -0000
To: dev@tomcat.apache.org
From: markt@apache.org
X-Mailer: svnmailer-1.0.9
Message-Id: <20140317214449.743A823889CB@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

Author: markt
Date: Mon Mar 17 21:44:48 2014
New Revision: 1578611

URL: http://svn.apache.org/r1578611
Log:
Prevent user supplied XSLTs from defining external entities

Modified:
    tomcat/trunk/conf/web.xml
    tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
    tomcat/trunk/java/org/apache/catalina/servlets/LocalStrings.properties
    tomcat/trunk/webapps/docs/default-servlet.xml

Modified: tomcat/trunk/conf/web.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/conf/web.xml?rev=1578611&r1=1578610&r2=1578611&view=diff
==============================================================================
--- tomcat/trunk/conf/web.xml (original)
+++ tomcat/trunk/conf/web.xml Mon Mar 17 21:44:48 2014
@@ -88,10 +88,12 @@
   <!--                       globalXsltFile[null]                           -->
   <!--                                                                      -->
   <!--   globalXsltFile      Site wide configuration version of             -->
-  <!--                       localXsltFile. This argument must be a         -->
-  <!--                       relative path that points to a location below  -->
-  <!--                       either $CATALINA_BASE/conf (checked first)     -->
-  <!--                       or $CATALINA_BASE/conf (checked second).[null] -->
+  <!--                       localXsltFile. This argument must either be an -->
+  <!--                       absolute or relative (to either                -->
+  <!--                       $CATALINA_BASE/conf or $CATALINA_HOME/conf)    -->
+  <!--                       path that points to a location below either    -->
+  <!--                       $CATALINA_BASE/conf (checked first) or         -->
+  <!--                       $CATALINA_HOME/conf (checked second).[null]    -->
 
     <servlet>
         <servlet-name>default</servlet-name>

Modified: tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java?rev=1578611&r1=1578610&r2=1578611&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java Mon Mar 17 21:44:48 2014
@@ -47,10 +47,14 @@ import javax.servlet.UnavailableExceptio
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.Source;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
 
@@ -64,6 +68,10 @@ import org.apache.catalina.util.RequestU
 import org.apache.catalina.util.ServerInfo;
 import org.apache.catalina.util.URLEncoder;
 import org.apache.tomcat.util.res.StringManager;
+import org.w3c.dom.Document;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.ext.EntityResolver2;
 
 
 /**
@@ -122,6 +130,11 @@ public class DefaultServlet extends Http
      */
     protected static final URLEncoder urlEncoder;
 
+    private static final DocumentBuilderFactory factory;
+
+    private static final SecureEntityResolver secureEntityResolver =
+            new SecureEntityResolver();
+
     /**
      * Full range marker.
      */
@@ -152,6 +165,10 @@ public class DefaultServlet extends Http
         urlEncoder.addSafeCharacter('.');
         urlEncoder.addSafeCharacter('*');
         urlEncoder.addSafeCharacter('/');
+
+        factory = DocumentBuilderFactory.newInstance();
+        factory.setNamespaceAware(true);
+        factory.setValidating(false);
     }
 
 
@@ -1192,13 +1209,12 @@ public class DefaultServlet extends Http
     protected InputStream render(String contextPath, WebResource resource)
         throws IOException, ServletException {
 
-        InputStream xsltInputStream =
-            findXsltInputStream(resource);
+        Source xsltSource = findXsltSource(resource);
 
-        if (xsltInputStream==null) {
+        if (xsltSource == null) {
             return renderHtml(contextPath, resource);
         }
-        return renderXml(contextPath, resource, xsltInputStream);
+        return renderXml(contextPath, resource, xsltSource);
 
     }
 
@@ -1211,7 +1227,7 @@ public class DefaultServlet extends Http
      */
     protected InputStream renderXml(String contextPath,
                                     WebResource resource,
-                                    InputStream xsltInputStream)
+                                    Source xsltSource)
         throws IOException, ServletException {
 
         StringBuilder sb = new StringBuilder();
@@ -1292,8 +1308,7 @@ public class DefaultServlet extends Http
         try {
             TransformerFactory tFactory = TransformerFactory.newInstance();
             Source xmlSource = new StreamSource(new StringReader(sb.toString()));
-            Source xslSource = new StreamSource(xsltInputStream);
-            Transformer transformer = tFactory.newTransformer(xslSource);
+            Transformer transformer = tFactory.newTransformer(xsltSource);
 
             ByteArrayOutputStream stream = new ByteArrayOutputStream();
             OutputStreamWriter osWriter = new OutputStreamWriter(stream, "UTF8");
@@ -1496,9 +1511,9 @@ public class DefaultServlet extends Http
 
 
     /**
-     * Return the xsl template inputstream (if possible)
+     * Return a Source for the xsl template (if possible)
      */
-    protected InputStream findXsltInputStream(WebResource directory)
+    protected Source findXsltSource(WebResource directory)
         throws IOException {
 
         if (localXsltFile != null) {
@@ -1507,7 +1522,11 @@ public class DefaultServlet extends Http
             if (resource.isFile()) {
                 InputStream is = resource.getInputStream();
                 if (is != null) {
-                    return is;
+                    if (Globals.IS_SECURITY_ENABLED) {
+                        return secureXslt(is);
+                    } else {
+                        return new StreamSource(is);
+                    }
                 }
             }
             if (debug > 10) {
@@ -1518,8 +1537,13 @@ public class DefaultServlet extends Http
         if (contextXsltFile != null) {
             InputStream is =
                 getServletContext().getResourceAsStream(contextXsltFile);
-            if (is != null)
-                return is;
+            if (is != null) {
+                if (Globals.IS_SECURITY_ENABLED) {
+                    return secureXslt(is);
+                } else {
+                    return new StreamSource(is);
+                }
+            }
 
             if (debug > 10)
                 log("contextXsltFile '" + contextXsltFile + "' not found");
@@ -1530,11 +1554,11 @@ public class DefaultServlet extends Http
          */
         if (globalXsltFile != null) {
             File f = validateGlobalXsltFile();
-            if (f != null && f.exists()){
+            if (f != null){
                 try (FileInputStream fis = new FileInputStream(f)){
                     byte b[] = new byte[(int)f.length()]; /* danger! */
                     fis.read(b);
-                    return new ByteArrayInputStream(b);
+                    return new StreamSource(new ByteArrayInputStream(b));
                 }
             }
         }
@@ -1550,7 +1574,9 @@ public class DefaultServlet extends Http
         File result = validateGlobalXsltFile(baseConf);
         if (result == null) {
             File homeConf = new File(context.getCatalinaHome(), "conf");
-            result = validateGlobalXsltFile(homeConf);
+            if (!baseConf.equals(homeConf)) {
+                result = validateGlobalXsltFile(homeConf);
+            }
         }
 
         return result;
@@ -1558,7 +1584,14 @@ public class DefaultServlet extends Http
 
 
     private File validateGlobalXsltFile(File base) {
-        File candidate = new File(base, globalXsltFile);
+        File candidate = new File(globalXsltFile);
+        if (!candidate.isAbsolute()) {
+            candidate = new File(base, globalXsltFile);
+        }
+
+        if (!candidate.isFile()) {
+            return null;
+        }
 
         // First check that the resulting path is under the provided base
         try {
@@ -1569,9 +1602,9 @@ public class DefaultServlet extends Http
             return null;
         }
 
-        // Next check that an .xlt or .xslt file has been specified
+        // Next check that an .xsl or .xslt file has been specified
         String nameLower = candidate.getName().toLowerCase(Locale.ENGLISH);
-        if (!nameLower.endsWith(".xslt") && !nameLower.endsWith(".xlt")) {
+        if (!nameLower.endsWith(".xslt") && !nameLower.endsWith(".xsl")) {
             return null;
         }
 
@@ -1579,8 +1612,32 @@ public class DefaultServlet extends Http
     }
 
 
-    // -------------------------------------------------------- protected Methods
+    private Source secureXslt(InputStream is) {
+        // Need to filter out any external entities
+        Source result = null;
+        try {
+            DocumentBuilder builder = factory.newDocumentBuilder();
+            builder.setEntityResolver(secureEntityResolver);
+            Document document = builder.parse(is);
+            result = new DOMSource(document);
+        } catch (ParserConfigurationException | SAXException | IOException e) {
+            if (debug > 0) {
+                log(e.getMessage(), e);
+            }
+        } finally {
+            if (is != null) {
+                try {
+                    is.close();
+                } catch (IOException e) {
+                    // Ignore
+                }
+            }
+        }
+        return result;
+    }
+
 
+    // -------------------------------------------------------- protected Methods
 
     /**
      * Check if sendfile can be used.
@@ -2074,9 +2131,6 @@ public class DefaultServlet extends Http
     }
 
 
-    // ------------------------------------------------------ Range Inner Class
-
-
     protected static class Range {
 
         public long start;
@@ -2092,4 +2146,34 @@ public class DefaultServlet extends Http
             return (start >= 0) && (end >= 0) && (start <= end) && (length > 0);
         }
     }
+
+
+    /**
+     * This is secure in the sense that any attempt to use an external entity
+     * will trigger an exception.
+     */
+    private static class SecureEntityResolver implements EntityResolver2  {
+
+        @Override
+        public InputSource resolveEntity(String publicId, String systemId)
+                throws SAXException, IOException {
+            throw new SAXException(sm.getString("defaultServlet.blockExternalEntity",
+                    publicId, systemId));
+        }
+
+        @Override
+        public InputSource getExternalSubset(String name, String baseURI)
+                throws SAXException, IOException {
+            throw new SAXException(sm.getString("defaultServlet.blockExternalSubset",
+                    name, baseURI));
+        }
+
+        @Override
+        public InputSource resolveEntity(String name, String publicId,
+                String baseURI, String systemId) throws SAXException,
+                IOException {
+            throw new SAXException(sm.getString("defaultServlet.blockExternalEntity2",
+                    name, publicId, baseURI, systemId));
+        }
+    }
 }

Modified: tomcat/trunk/java/org/apache/catalina/servlets/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/LocalStrings.properties?rev=1578611&r1=1578610&r2=1578611&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/servlets/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/servlets/LocalStrings.properties Mon Mar 17 21:44:48 2014
@@ -13,6 +13,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+defaultServlet.blockExternalEntity=Blocked access to external entity with publicId [{0}] and systemId [{0}]
+defaultServlet.blockExternalEntity2=Blocked access to external entity with name [{0}], publicId [{1}], baseURI [{2}] and systemId [{3}]
+defaultServlet.blockExternalSubset=Blocked access to external subset with name [{0}] and baseURI [{1}]
 defaultServlet.missingResource=The requested resource ({0}) is not available
 defaultservlet.skipfail=Only skipped [{0}] bytes when [{1}] were requested
 webdavservlet.jaxpfailed=JAXP initialization failed

Modified: tomcat/trunk/webapps/docs/default-servlet.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/default-servlet.xml?rev=1578611&r1=1578610&r2=1578611&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/default-servlet.xml (original)
+++ tomcat/trunk/webapps/docs/default-servlet.xml Mon Mar 17 21:44:48 2014
@@ -120,20 +120,22 @@ directory listings are disabled and debu
   </property>
   <property name="contextXsltFile">
         You may also customize your directory listing by context by
-        configuring <code>contextXsltFile</code>. This should be a context
-        relative path (e.g.: <code>/path/to/context.xslt</code>). This
-        overrides <code>globalXsltFile</code>. If this value is present but a
-        file does not exist, then <code>globalXsltFile</code> will be used. If
+        configuring <code>contextXsltFile</code>. This must be a context
+        relative path (e.g.: <code>/path/to/context.xslt</code>) to a file with
+        a <code>.xsl</code> or <code>.xslt</code> extension. This overrides
+        <code>globalXsltFile</code>. If this value is present but a file does
+        not exist, then <code>globalXsltFile</code> will be used. If
         <code>globalXsltFile</code> does not exist, then the default
         directory listing will be shown.
   </property>
   <property name="localXsltFile">
         You may also customize your directory listing by directory by
-        configuring <code>localXsltFile</code>. This should be a relative
-        file name in the directory where the listing will take place.
-        This overrides <code>globalXsltFile</code> and
-        <code>contextXsltFile</code>. If this value is present but a file
-        does not exist, then <code>contextXsltFile</code> will be used. If
+        configuring <code>localXsltFile</code>. This must be a file in the
+        directory where the listing will take place to with a
+        <code>.xsl</code> or <code>.xslt</code> extension. This overrides
+        <code>globalXsltFile</code> and <code>contextXsltFile</code>. If this
+        value is present but a file does not exist, then
+        <code>contextXsltFile</code> will be used. If
         <code>contextXsltFile</code> does not exist, then
         <code>globalXsltFile</code> will be used. If
         <code>globalXsltFile</code> does not exist, then the default



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org