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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
