Skip to content

Commit

Permalink
Defensive coding around some XML activities that are triggered by web…
Browse files Browse the repository at this point in the history
… applications and are therefore at potential risk of a memory leak.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk@1590028 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
markt-asf committed Apr 25, 2014
1 parent fea8fb3 commit 934f884
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 25 deletions.
30 changes: 28 additions & 2 deletions java/org/apache/catalina/servlets/DefaultServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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;
Expand Down Expand Up @@ -74,6 +75,8 @@
import org.apache.naming.resources.Resource;
import org.apache.naming.resources.ResourceAttributes;
import org.apache.tomcat.util.res.StringManager;
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;
Expand Down Expand Up @@ -1367,11 +1370,27 @@ protected InputStream renderXml(String contextPath,
sb.append("]]></readme>");
}


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());
}

TransformerFactory tFactory = TransformerFactory.newInstance();
Source xmlSource = new StreamSource(new StringReader(sb.toString()));
Transformer transformer = tFactory.newTransformer(xsltSource);
Expand All @@ -1384,6 +1403,13 @@ protected InputStream renderXml(String contextPath,
return (new ByteArrayInputStream(stream.toByteArray()));
} catch (TransformerException 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);
}
}
}

Expand Down
74 changes: 51 additions & 23 deletions java/org/apache/jasper/compiler/JspDocumentParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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;
Expand All @@ -35,6 +36,8 @@
import org.apache.jasper.JspCompilationContext;
import org.apache.tomcat.util.descriptor.DigesterFactory;
import org.apache.tomcat.util.descriptor.LocalResolver;
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;
Expand Down Expand Up @@ -1464,33 +1467,58 @@ private static SAXParser getSAXParser(
JspDocumentParser jspDocParser)
throws Exception {

SAXParserFactory factory = SAXParserFactory.newInstance();
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());
}

factory.setNamespaceAware(true);
// Preserve xmlns attributes
factory.setFeature(
"http://xml.org/sax/features/namespace-prefixes",
true);
SAXParserFactory factory = SAXParserFactory.newInstance();

factory.setValidating(validating);
if (validating) {
// Enable DTD validation
factory.setFeature(
"http://xml.org/sax/features/validation",
true);
// Enable schema validation
factory.setNamespaceAware(true);
// Preserve xmlns attributes
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);
"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);
}

return saxParser;
// 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);
}
}
}

/*
Expand Down
5 changes: 5 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@
patterns of the form <code>*.a.b</code> which are not valid patterns for
extension mappings. (markt)
</add>
<add>
Extend XML factory, parser etc. memory leak protection to cover some
additional locations where, theoretically, a memory leak could occur.
(markt)
</add>
</changelog>
</subsection>
<subsection name="Coyote">
Expand Down

0 comments on commit 934f884

Please sign in to comment.