You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/03/18 00:04:45 UTC

svn commit: r1578655 - in /tomcat/tc7.0.x/trunk: ./ conf/web.xml java/org/apache/catalina/servlets/DefaultServlet.java java/org/apache/catalina/servlets/LocalStrings.properties webapps/docs/default-servlet.xml

Author: markt
Date: Mon Mar 17 23:04:45 2014
New Revision: 1578655

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

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/conf/web.xml
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/LocalStrings.properties
    tomcat/tc7.0.x/trunk/webapps/docs/default-servlet.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1578611

Modified: tomcat/tc7.0.x/trunk/conf/web.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/conf/web.xml?rev=1578655&r1=1578654&r2=1578655&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/conf/web.xml (original)
+++ tomcat/tc7.0.x/trunk/conf/web.xml Mon Mar 17 23:04:45 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/tc7.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java?rev=1578655&r1=1578654&r2=1578655&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java Mon Mar 17 23:04:45 2014
@@ -52,10 +52,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;
 
@@ -70,6 +74,10 @@ import org.apache.naming.resources.Proxy
 import org.apache.naming.resources.Resource;
 import org.apache.naming.resources.ResourceAttributes;
 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;
 
 
 /**
@@ -119,9 +127,14 @@ public class DefaultServlet
 
     private static final long serialVersionUID = 1L;
 
-    // ----------------------------------------------------- Instance Variables
+    private static final DocumentBuilderFactory factory;
+
+    private static final SecureEntityResolver secureEntityResolver =
+            new SecureEntityResolver();
 
 
+    // ----------------------------------------------------- Instance Variables
+
     /**
      * The debugging detail level for this servlet.
      */
@@ -224,6 +237,10 @@ public class DefaultServlet
         urlEncoder.addSafeCharacter('.');
         urlEncoder.addSafeCharacter('*');
         urlEncoder.addSafeCharacter('/');
+        
+        factory = DocumentBuilderFactory.newInstance();
+        factory.setNamespaceAware(true);
+        factory.setValidating(false);
     }
 
 
@@ -1233,23 +1250,22 @@ public class DefaultServlet
     }
 
 
-
     /**
      *  Decide which way to render. HTML or XML.
      */
     protected InputStream render(String contextPath, CacheEntry cacheEntry)
         throws IOException, ServletException {
 
-        InputStream xsltInputStream =
-            findXsltInputStream(cacheEntry.context);
+        Source xsltSource = findXsltInputStream(cacheEntry.context);
 
-        if (xsltInputStream==null) {
+        if (xsltSource == null) {
             return renderHtml(contextPath, cacheEntry);
         }
-        return renderXml(contextPath, cacheEntry, xsltInputStream);
+        return renderXml(contextPath, cacheEntry, xsltSource);
 
     }
 
+    
     /**
      * Return an InputStream to an HTML representation of the contents
      * of this directory.
@@ -1259,7 +1275,7 @@ public class DefaultServlet
      */
     protected InputStream renderXml(String contextPath,
                                     CacheEntry cacheEntry,
-                                    InputStream xsltInputStream)
+                                    Source xsltSource)
         throws IOException, ServletException {
 
         StringBuilder sb = new StringBuilder();
@@ -1353,8 +1369,7 @@ public class DefaultServlet
         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");
@@ -1573,9 +1588,9 @@ public class DefaultServlet
 
 
     /**
-     * Return the xsl template inputstream (if possible)
+     * Return a Source for the xsl template (if possible)
      */
-    protected InputStream findXsltInputStream(DirContext directory)
+    protected Source findXsltInputStream(DirContext directory)
         throws IOException {
 
         if (localXsltFile != null) {
@@ -1583,8 +1598,13 @@ public class DefaultServlet
                 Object obj = directory.lookup(localXsltFile);
                 if ((obj != null) && (obj instanceof Resource)) {
                     InputStream is = ((Resource) obj).streamContent();
-                    if (is != null)
-                        return is;
+                    if (is != null) {
+                        if (Globals.IS_SECURITY_ENABLED) {
+                            return secureXslt(is);
+                        } else {
+                            return new StreamSource(is);
+                        }
+                    }
                 }
             } catch (NamingException e) {
                 if (debug > 10)
@@ -1595,8 +1615,13 @@ public class DefaultServlet
         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");
@@ -1607,13 +1632,13 @@ public class DefaultServlet
          */
         if (globalXsltFile != null) {
             File f = validateGlobalXsltFile();
-            if (f != null && f.exists()){
+            if (f != null){
                 FileInputStream fis = null;
                 try {
                     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));
                 } finally {
                     if (fis != null) {
                         try {
@@ -1643,7 +1668,7 @@ public class DefaultServlet
         
         if (result == null) {
             String home = System.getProperty(Globals.CATALINA_HOME_PROP);
-            if (home != null) {
+            if (home != null && !home.equals(base)) {
                 File homeConf = new File(home, "conf");
                 result = validateGlobalXsltFile(homeConf);
             }
@@ -1654,7 +1679,14 @@ public class DefaultServlet
 
 
     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 {
@@ -1665,9 +1697,9 @@ public class DefaultServlet
             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;
         }
 
@@ -1675,9 +1707,41 @@ public class DefaultServlet
     }
 
 
-    // -------------------------------------------------------- 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 e) {
+            if (debug > 0) {
+                log(e.getMessage(), e);
+            }
+        } catch (SAXException e) {
+            if (debug > 0) {
+                log(e.getMessage(), e);
+            }
+        } catch (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.
      */
@@ -2177,9 +2241,6 @@ public class DefaultServlet
     }
 
 
-    // ------------------------------------------------------ Range Inner Class
-
-
     protected static class Range {
 
         public long start;
@@ -2195,4 +2256,34 @@ public class DefaultServlet
             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/tc7.0.x/trunk/java/org/apache/catalina/servlets/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/LocalStrings.properties?rev=1578655&r1=1578654&r2=1578655&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/LocalStrings.properties (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/servlets/LocalStrings.properties Mon Mar 17 23:04:45 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.directorylistingfor=Directory Listing for:
 defaultservlet.upto=Up to:

Modified: tomcat/tc7.0.x/trunk/webapps/docs/default-servlet.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/default-servlet.xml?rev=1578655&r1=1578654&r2=1578655&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/default-servlet.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/default-servlet.xml Mon Mar 17 23:04:45 2014
@@ -122,10 +122,11 @@ The DefaultServlet allows the following 
     <th valign='top'>contextXsltFile</th>
     <td valign='top'>
         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.
     </td>
@@ -134,11 +135,12 @@ The DefaultServlet allows the following 
     <th valign='top'>localXsltFile</th>
     <td valign='top'>
         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