[jbossweb-commits] JBossWeb SVN: r2427 - in branches/7.4.x/src/main/java/org: apache/jasper/compiler and 3 other directories.

Author: remy.maucherat(a)jboss.com Date: 2014-05-28 04:20:59 -0400 (Wed, 28 May 2014) New Revision: 2427 Added: branches/7.4.x/src/main/java/org/apache/tomcat/util/security/ branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedGetTccl.java branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedSetTccl.java Modified: branches/7.4.x/src/main/java/org/apache/catalina/servlets/DefaultServlet.java branches/7.4.x/src/main/java/org/apache/jasper/compiler/JspDocumentParser.java branches/7.4.x/src/main/java/org/jboss/web/CatalinaMessages.java Log: Port fixes for CVE-2014-0119 (low) and CVE-2014-0096: Issue using global XSLT in the default servlet (not used in EAP), and leak issue using XML parsing in Jasper. Modified: branches/7.4.x/src/main/java/org/apache/catalina/servlets/DefaultServlet.java =================================================================== --- branches/7.4.x/src/main/java/org/apache/catalina/servlets/DefaultServlet.java 2014-05-28 07:21:40 UTC (rev 2426) +++ branches/7.4.x/src/main/java/org/apache/catalina/servlets/DefaultServlet.java 2014-05-28 08:20:59 UTC (rev 2427) @@ -36,8 +36,10 @@ import java.io.Reader; import java.io.StringReader; import java.io.StringWriter; +import java.security.AccessController; import java.util.ArrayList; import java.util.Iterator; +import java.util.Locale; import java.util.StringTokenizer; import javax.naming.InitialContext; @@ -53,10 +55,14 @@ 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 +76,12 @@ import org.apache.naming.resources.ProxyDirContext; import org.apache.naming.resources.Resource; import org.apache.naming.resources.ResourceAttributes; +import org.apache.tomcat.util.security.PrivilegedGetTccl; +import org.apache.tomcat.util.security.PrivilegedSetTccl; +import org.w3c.dom.Document; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.ext.EntityResolver2; /** @@ -84,7 +96,10 @@ public class DefaultServlet extends HttpServlet { + private static final DocumentBuilderFactory factory; + private static final SecureEntityResolver secureEntityResolver; + // ----------------------------------------------------- Instance Variables @@ -190,6 +205,16 @@ urlEncoder.addSafeCharacter('.'); urlEncoder.addSafeCharacter('*'); urlEncoder.addSafeCharacter('/'); + + if (Globals.IS_SECURITY_ENABLED) { + factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + factory.setValidating(false); + secureEntityResolver = new SecureEntityResolver(); + } else { + factory = null; + secureEntityResolver = null; + } } @@ -1083,17 +1108,12 @@ * Decide which way to render. HTML or XML. */ protected InputStream render(String contextPath, CacheEntry cacheEntry) - throws IOException, ServletException { - - InputStream xsltInputStream = - findXsltInputStream(cacheEntry.context); - - if (xsltInputStream==null) { + throws IOException, ServletException { + Source xsltSource = findXsltInputStream(cacheEntry.context); + if (xsltSource == null) { return renderHtml(contextPath, cacheEntry); - } else { - return renderXml(contextPath, cacheEntry, xsltInputStream); } - + return renderXml(contextPath, cacheEntry, xsltSource); } /** @@ -1105,7 +1125,7 @@ */ protected InputStream renderXml(String contextPath, CacheEntry cacheEntry, - InputStream xsltInputStream) + Source xsltSource) throws IOException, ServletException { StringBuilder sb = new StringBuilder(); @@ -1196,12 +1216,28 @@ sb.append("</listing>"); + // Prevent possible memory leak. Ensure Transformer and + // TransformerFactory are not loaded from the web application. + ClassLoader original; + if (Globals.IS_SECURITY_ENABLED) { + PrivilegedGetTccl pa = new PrivilegedGetTccl(); + original = AccessController.doPrivileged(pa); + } else { + original = Thread.currentThread().getContextClassLoader(); + } + try { + if (Globals.IS_SECURITY_ENABLED) { + PrivilegedSetTccl pa = + new PrivilegedSetTccl(DefaultServlet.class.getClassLoader()); + AccessController.doPrivileged(pa); + } else { + Thread.currentThread().setContextClassLoader( + DefaultServlet.class.getClassLoader()); + } - 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"); @@ -1210,7 +1246,14 @@ osWriter.flush(); return (new ByteArrayInputStream(stream.toByteArray())); } catch (TransformerException e) { - throw new ServletException(MESSAGES.xslTransformerError(), e); + throw new ServletException("XSL transformer error", e); + } finally { + if (Globals.IS_SECURITY_ENABLED) { + PrivilegedSetTccl pa = new PrivilegedSetTccl(original); + AccessController.doPrivileged(pa); + } else { + Thread.currentThread().setContextClassLoader(original); + } } } @@ -1429,7 +1472,7 @@ /** * Return the xsl template inputstream (if possible) */ - protected InputStream findXsltInputStream(DirContext directory) + protected Source findXsltInputStream(DirContext directory) throws IOException, ServletException { if (localXsltFile != null) { @@ -1437,8 +1480,13 @@ 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) @@ -1449,8 +1497,13 @@ 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"); @@ -1459,20 +1512,24 @@ /* Open and read in file in one fell swoop to reduce chance * chance of leaving handle open. */ - if (globalXsltFile!=null) { - FileInputStream fis = null; - - try { - File f = new File(globalXsltFile); - if (f.exists()){ - fis =new FileInputStream(f); + if (globalXsltFile != null) { + File f = validateGlobalXsltFile(); + 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 { + fis.close(); + } catch (IOException ioe) { + // Ignore + } + } } - } finally { - if (fis!=null) - fis.close(); } } @@ -1481,6 +1538,90 @@ } + private File validateGlobalXsltFile() { + + File result = null; + String base = System.getProperty("catalina.base"); + + if (base != null) { + File baseConf = new File(base, "conf"); + result = validateGlobalXsltFile(baseConf); + } + + if (result == null) { + String home = System.getProperty("catalina.home"); + if (home != null && !home.equals(base)) { + File homeConf = new File(home, "conf"); + result = validateGlobalXsltFile(homeConf); + } + } + + return result; + } + + + private File validateGlobalXsltFile(File base) { + 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 { + if (!candidate.getCanonicalPath().startsWith(base.getCanonicalPath())) { + return null; + } + } catch (IOException ioe) { + return null; + } + + // Next check that an .xsl or .xslt file has been specified + String nameLower = candidate.getName().toLowerCase(Locale.ENGLISH); + if (!nameLower.endsWith(".xslt") && !nameLower.endsWith(".xsl")) { + return null; + } + + return candidate; + } + + + 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 @@ -2152,4 +2293,29 @@ } + /** + * 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(MESSAGES.ignoredExternalEntity(publicId, systemId)); + } + + @Override + public InputSource getExternalSubset(String name, String baseURI) + throws SAXException, IOException { + throw new SAXException(MESSAGES.ignoredExternalSubset(name, baseURI)); + } + + @Override + public InputSource resolveEntity(String name, String publicId, + String baseURI, String systemId) throws SAXException, + IOException { + throw new SAXException(MESSAGES.ignoredExternalEntity(name, publicId, baseURI, systemId)); + } + } } Modified: branches/7.4.x/src/main/java/org/apache/jasper/compiler/JspDocumentParser.java =================================================================== --- branches/7.4.x/src/main/java/org/apache/jasper/compiler/JspDocumentParser.java 2014-05-28 07:21:40 UTC (rev 2426) +++ branches/7.4.x/src/main/java/org/apache/jasper/compiler/JspDocumentParser.java 2014-05-28 08:20:59 UTC (rev 2427) @@ -22,7 +22,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; - +import java.security.AccessController; import java.util.Iterator; import java.util.List; import java.util.jar.JarFile; @@ -33,8 +33,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.security.PrivilegedGetTccl; +import org.apache.tomcat.util.security.PrivilegedSetTccl; import org.xml.sax.Attributes; import org.xml.sax.InputSource; import org.xml.sax.Locator; @@ -1365,30 +1368,59 @@ * * @return The SAXParser */ - private static SAXParser getSAXParser( - boolean validating, - JspDocumentParser jspDocParser) - throws Exception { + private static SAXParser getSAXParser(boolean validating, + JspDocumentParser jspDocParser) throws Exception { - SAXParserFactory factory = SAXParserFactory.newInstance(); - factory.setNamespaceAware(true); + ClassLoader original; + if (Constants.IS_SECURITY_ENABLED) { + PrivilegedGetTccl pa = new PrivilegedGetTccl(); + original = AccessController.doPrivileged(pa); + } else { + original = Thread.currentThread().getContextClassLoader(); + } + try { + if (Constants.IS_SECURITY_ENABLED) { + PrivilegedSetTccl pa = new PrivilegedSetTccl( + JspDocumentParser.class.getClassLoader()); + AccessController.doPrivileged(pa); + } else { + Thread.currentThread().setContextClassLoader( + JspDocumentParser.class.getClassLoader()); + } - // Preserve xmlns attributes - factory.setFeature( - "http://xml.org/sax/features/namespace-prefixes", - true); - factory.setValidating(validating); - //factory.setFeature( - // "http://xml.org/sax/features/validation", - // validating); - - // Configure the parser - SAXParser saxParser = factory.newSAXParser(); - XMLReader xmlReader = saxParser.getXMLReader(); - xmlReader.setProperty(LEXICAL_HANDLER_PROPERTY, jspDocParser); - xmlReader.setErrorHandler(jspDocParser); + SAXParserFactory factory = SAXParserFactory.newInstance(); - return saxParser; + factory.setNamespaceAware(true); + // Preserve xmlns attributes + factory.setFeature( + "http://xml.org/sax/features/namespace-prefixes", true); + + factory.setValidating(validating); + if (validating) { + // Enable DTD validation + factory.setFeature("http://xml.org/sax/features/validation";, + true); + // Enable schema validation + factory.setFeature( + "http://apache.org/xml/features/validation/schema", + true); + } + + // Configure the parser + SAXParser saxParser = factory.newSAXParser(); + XMLReader xmlReader = saxParser.getXMLReader(); + xmlReader.setProperty(LEXICAL_HANDLER_PROPERTY, jspDocParser); + xmlReader.setErrorHandler(jspDocParser); + + return saxParser; + } finally { + if (Constants.IS_SECURITY_ENABLED) { + PrivilegedSetTccl pa = new PrivilegedSetTccl(original); + AccessController.doPrivileged(pa); + } else { + Thread.currentThread().setContextClassLoader(original); + } + } } /* Added: branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedGetTccl.java =================================================================== --- branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedGetTccl.java (rev 0) +++ branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedGetTccl.java 2014-05-28 08:20:59 UTC (rev 2427) @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.security; + +import java.security.PrivilegedAction; + +public class PrivilegedGetTccl implements PrivilegedAction<ClassLoader> { + @Override + public ClassLoader run() { + return Thread.currentThread().getContextClassLoader(); + } +} + + Added: branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedSetTccl.java =================================================================== --- branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedSetTccl.java (rev 0) +++ branches/7.4.x/src/main/java/org/apache/tomcat/util/security/PrivilegedSetTccl.java 2014-05-28 08:20:59 UTC (rev 2427) @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.security; + +import java.security.PrivilegedAction; + +public class PrivilegedSetTccl implements PrivilegedAction<Void> { + + private ClassLoader cl; + + public PrivilegedSetTccl(ClassLoader cl) { + this.cl = cl; + } + + @Override + public Void run() { + Thread.currentThread().setContextClassLoader(cl); + return null; + } +} \ No newline at end of file Modified: branches/7.4.x/src/main/java/org/jboss/web/CatalinaMessages.java =================================================================== --- branches/7.4.x/src/main/java/org/jboss/web/CatalinaMessages.java 2014-05-28 07:21:40 UTC (rev 2426) +++ branches/7.4.x/src/main/java/org/jboss/web/CatalinaMessages.java 2014-05-28 08:20:59 UTC (rev 2427) @@ -1023,4 +1023,10 @@ @Message(id = 377, value = "Error getting keys") String errorGettingKeys(); + @Message(id = 378, value = "Ignored external entity %s %s %s %s") + String ignoredExternalEntity(String name, String publicId, String baseUri, String systemId); + + @Message(id = 379, value = "Ignored external subset %s %s") + String ignoredExternalSubset(String publicId, String systemId); + }