Skip to content

Commit 3c53c4d

Browse files
markt-asfmarkt-asf
committed
Prevent user supplied XSLTs from defining external entities
git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk@1578655 13f79535-47bb-0310-9956-ffa450edef68
1 parent 5c545da commit 3c53c4d

File tree

4 files changed

+136
-38
lines changed

4 files changed

+136
-38
lines changed

‎conf/web.xml‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,12 @@
8888
<!-- globalXsltFile[null] -->
8989
<!-- -->
9090
<!-- globalXsltFile Site wide configuration version of -->
91-
<!-- localXsltFile. This argument must be a -->
92-
<!-- relative path that points to a location below -->
93-
<!-- either $CATALINA_BASE/conf (checked first) -->
94-
<!-- or $CATALINA_BASE/conf (checked second).[null] -->
91+
<!-- localXsltFile. This argument must either be an -->
92+
<!-- absolute or relative (to either -->
93+
<!-- $CATALINA_BASE/conf or $CATALINA_HOME/conf) -->
94+
<!-- path that points to a location below either -->
95+
<!-- $CATALINA_BASE/conf (checked first) or -->
96+
<!-- $CATALINA_HOME/conf (checked second).[null] -->
9597

9698
<servlet>
9799
<servlet-name>default</servlet-name>

‎java/org/apache/catalina/servlets/DefaultServlet.java‎

Lines changed: 116 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,14 @@
5252
import javax.servlet.http.HttpServlet;
5353
import javax.servlet.http.HttpServletRequest;
5454
import javax.servlet.http.HttpServletResponse;
55+
import javax.xml.parsers.DocumentBuilder;
56+
import javax.xml.parsers.DocumentBuilderFactory;
57+
import javax.xml.parsers.ParserConfigurationException;
5558
import javax.xml.transform.Source;
5659
import javax.xml.transform.Transformer;
5760
import javax.xml.transform.TransformerException;
5861
import javax.xml.transform.TransformerFactory;
62+
import javax.xml.transform.dom.DOMSource;
5963
import javax.xml.transform.stream.StreamResult;
6064
import javax.xml.transform.stream.StreamSource;
6165

@@ -70,6 +74,10 @@
7074
import org.apache.naming.resources.Resource;
7175
import org.apache.naming.resources.ResourceAttributes;
7276
import org.apache.tomcat.util.res.StringManager;
77+
import org.w3c.dom.Document;
78+
import org.xml.sax.InputSource;
79+
import org.xml.sax.SAXException;
80+
import org.xml.sax.ext.EntityResolver2;
7381

7482

7583
/**
@@ -119,8 +127,13 @@ public class DefaultServlet
119127

120128
private static final long serialVersionUID = 1L;
121129

122-
// ----------------------------------------------------- Instance Variables
130+
private static final DocumentBuilderFactory factory;
131+
132+
private static final SecureEntityResolver secureEntityResolver =
133+
new SecureEntityResolver();
134+
123135

136+
// ----------------------------------------------------- Instance Variables
124137

125138
/**
126139
* The debugging detail level for this servlet.
@@ -224,6 +237,10 @@ public class DefaultServlet
224237
urlEncoder.addSafeCharacter('.');
225238
urlEncoder.addSafeCharacter('*');
226239
urlEncoder.addSafeCharacter('/');
240+
241+
factory = DocumentBuilderFactory.newInstance();
242+
factory.setNamespaceAware(true);
243+
factory.setValidating(false);
227244
}
228245

229246

@@ -1233,23 +1250,22 @@ protected ArrayList<Range> parseRange(HttpServletRequest request,
12331250
}
12341251

12351252

1236-
12371253
/**
12381254
* Decide which way to render. HTML or XML.
12391255
*/
12401256
protected InputStream render(String contextPath, CacheEntry cacheEntry)
12411257
throws IOException, ServletException {
12421258

1243-
InputStream xsltInputStream =
1244-
findXsltInputStream(cacheEntry.context);
1259+
Source xsltSource = findXsltInputStream(cacheEntry.context);
12451260

1246-
if (xsltInputStream==null) {
1261+
if (xsltSource == null) {
12471262
return renderHtml(contextPath, cacheEntry);
12481263
}
1249-
return renderXml(contextPath, cacheEntry, xsltInputStream);
1264+
return renderXml(contextPath, cacheEntry, xsltSource);
12501265

12511266
}
12521267

1268+
12531269
/**
12541270
* Return an InputStream to an HTML representation of the contents
12551271
* of this directory.
@@ -1259,7 +1275,7 @@ protected InputStream render(String contextPath, CacheEntry cacheEntry)
12591275
*/
12601276
protected InputStream renderXml(String contextPath,
12611277
CacheEntry cacheEntry,
1262-
InputStream xsltInputStream)
1278+
Source xsltSource)
12631279
throws IOException, ServletException {
12641280

12651281
StringBuilder sb = new StringBuilder();
@@ -1353,8 +1369,7 @@ protected InputStream renderXml(String contextPath,
13531369
try {
13541370
TransformerFactory tFactory = TransformerFactory.newInstance();
13551371
Source xmlSource = new StreamSource(new StringReader(sb.toString()));
1356-
Source xslSource = new StreamSource(xsltInputStream);
1357-
Transformer transformer = tFactory.newTransformer(xslSource);
1372+
Transformer transformer = tFactory.newTransformer(xsltSource);
13581373

13591374
ByteArrayOutputStream stream = new ByteArrayOutputStream();
13601375
OutputStreamWriter osWriter = new OutputStreamWriter(stream, "UTF8");
@@ -1573,18 +1588,23 @@ protected String getReadme(DirContext directory)
15731588

15741589

15751590
/**
1576-
* Return the xsl template inputstream (if possible)
1591+
* Return a Source for the xsl template (if possible)
15771592
*/
1578-
protected InputStream findXsltInputStream(DirContext directory)
1593+
protected Source findXsltInputStream(DirContext directory)
15791594
throws IOException {
15801595

15811596
if (localXsltFile != null) {
15821597
try {
15831598
Object obj = directory.lookup(localXsltFile);
15841599
if ((obj != null) && (obj instanceof Resource)) {
15851600
InputStream is = ((Resource) obj).streamContent();
1586-
if (is != null)
1587-
return is;
1601+
if (is != null) {
1602+
if (Globals.IS_SECURITY_ENABLED) {
1603+
return secureXslt(is);
1604+
} else {
1605+
return new StreamSource(is);
1606+
}
1607+
}
15881608
}
15891609
} catch (NamingException e) {
15901610
if (debug > 10)
@@ -1595,8 +1615,13 @@ protected InputStream findXsltInputStream(DirContext directory)
15951615
if (contextXsltFile != null) {
15961616
InputStream is =
15971617
getServletContext().getResourceAsStream(contextXsltFile);
1598-
if (is != null)
1599-
return is;
1618+
if (is != null) {
1619+
if (Globals.IS_SECURITY_ENABLED) {
1620+
return secureXslt(is);
1621+
} else {
1622+
return new StreamSource(is);
1623+
}
1624+
}
16001625

16011626
if (debug > 10)
16021627
log("contextXsltFile '" + contextXsltFile + "' not found");
@@ -1607,13 +1632,13 @@ protected InputStream findXsltInputStream(DirContext directory)
16071632
*/
16081633
if (globalXsltFile != null) {
16091634
File f = validateGlobalXsltFile();
1610-
if (f != null && f.exists()){
1635+
if (f != null){
16111636
FileInputStream fis = null;
16121637
try {
16131638
fis = new FileInputStream(f);
16141639
byte b[] = new byte[(int)f.length()]; /* danger! */
16151640
fis.read(b);
1616-
return new ByteArrayInputStream(b);
1641+
return new StreamSource(new ByteArrayInputStream(b));
16171642
} finally {
16181643
if (fis != null) {
16191644
try {
@@ -1643,7 +1668,7 @@ private File validateGlobalXsltFile() {
16431668

16441669
if (result == null) {
16451670
String home = System.getProperty(Globals.CATALINA_HOME_PROP);
1646-
if (home != null) {
1671+
if (home != null && !home.equals(base)) {
16471672
File homeConf = new File(home, "conf");
16481673
result = validateGlobalXsltFile(homeConf);
16491674
}
@@ -1654,7 +1679,14 @@ private File validateGlobalXsltFile() {
16541679

16551680

16561681
private File validateGlobalXsltFile(File base) {
1657-
File candidate = new File(base, globalXsltFile);
1682+
File candidate = new File(globalXsltFile);
1683+
if (!candidate.isAbsolute()) {
1684+
candidate = new File(base, globalXsltFile);
1685+
}
1686+
1687+
if (!candidate.isFile()) {
1688+
return null;
1689+
}
16581690

16591691
// First check that the resulting path is under the provided base
16601692
try {
@@ -1665,19 +1697,51 @@ private File validateGlobalXsltFile(File base) {
16651697
return null;
16661698
}
16671699

1668-
// Next check that an .xlt or .xslt file has been specified
1700+
// Next check that an .xsl or .xslt file has been specified
16691701
String nameLower = candidate.getName().toLowerCase(Locale.ENGLISH);
1670-
if (!nameLower.endsWith(".xslt") && !nameLower.endsWith(".xlt")) {
1702+
if (!nameLower.endsWith(".xslt") && !nameLower.endsWith(".xsl")) {
16711703
return null;
16721704
}
16731705

16741706
return candidate;
16751707
}
16761708

16771709

1678-
// -------------------------------------------------------- protected Methods
1710+
private Source secureXslt(InputStream is) {
1711+
// Need to filter out any external entities
1712+
Source result = null;
1713+
try {
1714+
DocumentBuilder builder = factory.newDocumentBuilder();
1715+
builder.setEntityResolver(secureEntityResolver);
1716+
Document document = builder.parse(is);
1717+
result = new DOMSource(document);
1718+
} catch (ParserConfigurationException e) {
1719+
if (debug > 0) {
1720+
log(e.getMessage(), e);
1721+
}
1722+
} catch (SAXException e) {
1723+
if (debug > 0) {
1724+
log(e.getMessage(), e);
1725+
}
1726+
} catch (IOException e) {
1727+
if (debug > 0) {
1728+
log(e.getMessage(), e);
1729+
}
1730+
} finally {
1731+
if (is != null) {
1732+
try {
1733+
is.close();
1734+
} catch (IOException e) {
1735+
// Ignore
1736+
}
1737+
}
1738+
}
1739+
return result;
1740+
}
16791741

16801742

1743+
// -------------------------------------------------------- protected Methods
1744+
16811745
/**
16821746
* Check if sendfile can be used.
16831747
*/
@@ -2177,9 +2241,6 @@ protected IOException copyRange(InputStream istream,
21772241
}
21782242

21792243

2180-
// ------------------------------------------------------ Range Inner Class
2181-
2182-
21832244
protected static class Range {
21842245

21852246
public long start;
@@ -2195,4 +2256,34 @@ public boolean validate() {
21952256
return (start >= 0) && (end >= 0) && (start <= end) && (length > 0);
21962257
}
21972258
}
2259+
2260+
2261+
/**
2262+
* This is secure in the sense that any attempt to use an external entity
2263+
* will trigger an exception.
2264+
*/
2265+
private static class SecureEntityResolver implements EntityResolver2 {
2266+
2267+
@Override
2268+
public InputSource resolveEntity(String publicId, String systemId)
2269+
throws SAXException, IOException {
2270+
throw new SAXException(sm.getString("defaultServlet.blockExternalEntity",
2271+
publicId, systemId));
2272+
}
2273+
2274+
@Override
2275+
public InputSource getExternalSubset(String name, String baseURI)
2276+
throws SAXException, IOException {
2277+
throw new SAXException(sm.getString("defaultServlet.blockExternalSubset",
2278+
name, baseURI));
2279+
}
2280+
2281+
@Override
2282+
public InputSource resolveEntity(String name, String publicId,
2283+
String baseURI, String systemId) throws SAXException,
2284+
IOException {
2285+
throw new SAXException(sm.getString("defaultServlet.blockExternalEntity2",
2286+
name, publicId, baseURI, systemId));
2287+
}
2288+
}
21982289
}

‎java/org/apache/catalina/servlets/LocalStrings.properties‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
1515

16+
defaultServlet.blockExternalEntity=Blocked access to external entity with publicId [{0}] and systemId [{0}]
17+
defaultServlet.blockExternalEntity2=Blocked access to external entity with name [{0}], publicId [{1}], baseURI [{2}] and systemId [{3}]
18+
defaultServlet.blockExternalSubset=Blocked access to external subset with name [{0}] and baseURI [{1}]
1619
defaultServlet.missingResource=The requested resource ({0}) is not available
1720
defaultservlet.directorylistingfor=Directory Listing for:
1821
defaultservlet.upto=Up to:

‎webapps/docs/default-servlet.xml‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,11 @@ The DefaultServlet allows the following initParamters:
122122
<th valign='top'>contextXsltFile</th>
123123
<td valign='top'>
124124
You may also customize your directory listing by context by
125-
configuring <code>contextXsltFile</code>. This should be a context
126-
relative path (e.g.: <code>/path/to/context.xslt</code>). This
127-
overrides <code>globalXsltFile</code>. If this value is present but a
128-
file does not exist, then <code>globalXsltFile</code> will be used. If
125+
configuring <code>contextXsltFile</code>. This must be a context
126+
relative path (e.g.: <code>/path/to/context.xslt</code>) to a file with
127+
a <code>.xsl</code> or <code>.xslt</code> extension. This overrides
128+
<code>globalXsltFile</code>. If this value is present but a file does
129+
not exist, then <code>globalXsltFile</code> will be used. If
129130
<code>globalXsltFile</code> does not exist, then the default
130131
directory listing will be shown.
131132
</td>
@@ -134,11 +135,12 @@ The DefaultServlet allows the following initParamters:
134135
<th valign='top'>localXsltFile</th>
135136
<td valign='top'>
136137
You may also customize your directory listing by directory by
137-
configuring <code>localXsltFile</code>. This should be a relative
138-
file name in the directory where the listing will take place.
139-
This overrides <code>globalXsltFile</code> and
140-
<code>contextXsltFile</code>. If this value is present but a file
141-
does not exist, then <code>contextXsltFile</code> will be used. If
138+
configuring <code>localXsltFile</code>. This must be a file in the
139+
directory where the listing will take place to with a
140+
<code>.xsl</code> or <code>.xslt</code> extension. This overrides
141+
<code>globalXsltFile</code> and <code>contextXsltFile</code>. If this
142+
value is present but a file does not exist, then
143+
<code>contextXsltFile</code> will be used. If
142144
<code>contextXsltFile</code> does not exist, then
143145
<code>globalXsltFile</code> will be used. If
144146
<code>globalXsltFile</code> does not exist, then the default

0 commit comments

Comments
 (0)