Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-05 14:47:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

WW-5621 Harden XML parsers against Entity Expansion (Billion Laughs) attacks (#1643)

Browse Source 
Backport of apache/struts#1642 from Struts 7 to Struts 6.

Modern JDKs (7u45+) already protect against this attack with a built-in
64K entity expansion limit. These changes add defense-in-depth hardening
and remove unnecessary attack surface.

- Enable FEATURE_SECURE_PROCESSING in DomHelper SAX parser
- Enable FEATURE_SECURE_PROCESSING in DigesterDefinitionsReader
- Remove unused parseStringAsXML feature from StringAdapter to eliminate
  a theoretical XML Entity Expansion vector
- Deprecate setParseStringAsXML() and getParseStringAsXML() for removal
- Add Billion Laughs protection tests for DomHelper and DigesterDefinitionsReader

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Lukasz Lenart
2026-04-04 11:20:47 +02:00
committed by GitHub
parent 39c1e29958
commit b936fcbf8b
5 changed files with 77 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/com/opensymphony/xwork2/util/DomHelper.java
+2
View File
@@ -38,6 +38,7 @@ import org.xml.sax.SAXNotSupportedException;
import org.xml.sax.SAXParseException;
import org.xml.sax.helpers.DefaultHandler;
import javax.xml.XMLConstants;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;
@@ -104,6 +105,7 @@ public class DomHelper {
try {
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
} catch (ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) {
throw new StrutsException("Unable to disable resolving external entities!", e);
}
core/src/test/java/com/opensymphony/xwork2/util/DomHelperTest.java
+35 -3
View File
@@ -24,17 +24,18 @@ import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;
import org.xml.sax.SAXParseException;
import java.io.StringReader;
import java.util.Objects;
/**
* Test cases for {@link DomHelper}.
*/
public class DomHelperTest extends TestCase {
private final String xml = "<!DOCTYPE foo [<!ELEMENT foo (bar)><!ELEMENT bar (#PCDATA)>]>\n<foo>\n<bar/>\n</foo>\n";
public void testParse() {
String xml = "<!DOCTYPE foo [<!ELEMENT foo (bar)><!ELEMENT bar (#PCDATA)>]>\n<foo>\n<bar/>\n</foo>\n";
InputSource in = new InputSource(new StringReader(xml));
in.setSystemId("foo://bar");
@@ -47,6 +48,7 @@ public class DomHelperTest extends TestCase {
}
public void testGetLocationObject() {
String xml = "<!DOCTYPE foo [<!ELEMENT foo (bar)><!ELEMENT bar (#PCDATA)>]>\n<foo>\n<bar/>\n</foo>\n";
InputSource in = new InputSource(new StringReader(xml));
in.setSystemId("foo://bar");
@@ -61,7 +63,7 @@ public class DomHelperTest extends TestCase {
}
public void testExternalEntities() {
String dtdFile = getClass().getResource("/author.dtd").getPath();
String dtdFile = Objects.requireNonNull(getClass().getResource("/author.dtd")).getPath();
String xml = "<!DOCTYPE foo [<!ELEMENT foo (bar)><!ELEMENT bar (#PCDATA)><!ENTITY writer SYSTEM \"file://" + dtdFile + "\">]><foo><bar>&writer;</bar></foo>";
InputSource in = new InputSource(new StringReader(xml));
in.setSystemId("foo://bar");
@@ -74,4 +76,34 @@ public class DomHelperTest extends TestCase {
assertEquals(1, nl.getLength());
assertNull(nl.item(0).getNodeValue());
}
/**
* Tests that the parser is protected against Billion Laughs (XML Entity Expansion) attack.
* The FEATURE_SECURE_PROCESSING flag and the JDK's built-in entity expansion limit (64K
* since JDK 7u45) both cap entity expansion to prevent DoS.
* See: <a href="https://en.wikipedia.org/wiki/Billion_laughs_attack">Billion laughs attack</a>
*/
public void testBillionLaughsProtection() {
String xml = "<?xml version=\"1.0\"?>" +
"<!DOCTYPE root [" +
"<!ENTITY lol0 \"lol\">" +
"<!ENTITY lol1 \"&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;\">" +
"<!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">" +
"<!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">" +
"<!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">" +
"<!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">" +
"]>" +
"<root>&lol5;</root>";
InputSource in = new InputSource(new StringReader(xml));
in.setSystemId("test://billion-laughs");
try {
DomHelper.parse(in);
fail("Parser should reject excessive entity expansion");
} catch (Exception e) {
assertNotNull(e.getCause());
assertTrue(e.getCause() instanceof SAXParseException);
}
}
}
plugins/tiles/src/main/java/org/apache/tiles/core/definition/digester/DigesterDefinitionsReader.java
+3
View File
@@ -35,6 +35,7 @@ import org.xml.sax.SAXNotRecognizedException;
import org.xml.sax.SAXNotSupportedException;
import org.xml.sax.SAXParseException;
import javax.xml.XMLConstants;
import javax.xml.parsers.ParserConfigurationException;
import java.io.IOException;
import java.io.InputStream;
@@ -164,6 +165,8 @@ public class DigesterDefinitionsReader implements DefinitionsReader {
// Disable external DTDs as well
digester.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
digester.setXIncludeAware(false);
// Enable secure processing to limit entity expansion (prevents Billion Laughs attack)
digester.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
} catch (ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) {
throw new StrutsException("Unable to disable external XML entity parsing", e);
}
plugins/tiles/src/test/java/org/apache/tiles/core/definition/digester/TestDigesterDefinitionsReader.java
+23
View File
@@ -26,9 +26,11 @@ import org.apache.tiles.core.definition.DefinitionsFactoryException;
import org.junit.Before;
import org.junit.Test;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Map;
@@ -268,6 +270,27 @@ public class TestDigesterDefinitionsReader {
assertNull(reader.read(null));
}
/**
* Tests that the Digester parser is protected against Billion Laughs (XML Entity Expansion) attack.
* FEATURE_SECURE_PROCESSING is enabled in DigesterDefinitionsReader to limit entity expansion.
*/
@Test(expected = DefinitionsFactoryException.class)
public void testBillionLaughsProtection() {
String xml = "<?xml version=\"1.0\"?>" +
"<!DOCTYPE root [" +
"<!ENTITY lol0 \"lol\">" +
"<!ENTITY lol1 \"&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;&lol0;\">" +
"<!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">" +
"<!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">" +
"<!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">" +
"<!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">" +
"]>" +
"<root>&lol5;</root>";
InputStream source = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8));
reader.read(source);
}
/**
* Tests {@link DigesterDefinitionsReader#addDefinition(Definition)}.
*/
plugins/xslt/src/main/java/org/apache/struts2/result/xslt/StringAdapter.java
+14 -38
View File
@@ -18,13 +18,10 @@
*/
package org.apache.struts2.result.xslt;
import com.opensymphony.xwork2.util.DomHelper;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.w3c.dom.Node;
import org.xml.sax.InputSource;
import java.io.StringReader;
import java.util.ArrayList;
import java.util.List;
@@ -39,17 +36,13 @@ import java.util.List;
*
* <p>
* Subclasses may override the getStringValue() method in order to use StringAdapter
* as a simplified custom XML adapter for Java types. A subclass can enable XML
* parsing of the value string via the setParseStringAsXML() method and then
* override getStringValue() to return a String containing the custom formatted XML.
* as a simplified custom XML adapter for Java types.
* </p>
*/
public class StringAdapter extends AbstractAdapterElement {
private static final Logger LOG = LogManager.getLogger(StringAdapter.class);
boolean parseStringAsXML;
public StringAdapter() {
}
@@ -58,16 +51,11 @@ public class StringAdapter extends AbstractAdapterElement {
}
/**
* <p>
* Get the object to be adapted as a String value.
* </p>
*
* <p>
* This method can be overridden by subclasses that wish to use StringAdapter
* as a simplified customizable XML adapter for Java types. A subclass can
* enable parsing of the value string as containing XML text via the
* setParseStringAsXML() method and then override getStringValue() to return a
* String containing the custom formatted XML.
* as a simplified customizable XML adapter for Java types.
* </p>
*
* @return the string value
@@ -78,17 +66,8 @@ public class StringAdapter extends AbstractAdapterElement {
@Override
protected List<Node> buildChildAdapters() {
Node node;
if (getParseStringAsXML()) {
LOG.debug("parsing string as xml: {}", getStringValue());
// Parse the String to a DOM, then proxy that as our child
node = DomHelper.parse(new InputSource(new StringReader(getStringValue())));
node = getAdapterFactory().proxyNode(this, node);
} else {
LOG.debug("using string as is: {}", getStringValue());
// Create a Text node as our child
node = new SimpleTextNode(getAdapterFactory(), this, "text", getStringValue());
}
LOG.debug("using string as is: {}", getStringValue());
Node node = new SimpleTextNode(getAdapterFactory(), this, "text", getStringValue());
List<Node> children = new ArrayList<>();
children.add(node);
@@ -96,26 +75,23 @@ public class StringAdapter extends AbstractAdapterElement {
}
/**
* @return is this StringAdapter to interpret its string values as containing
* XML Text?
*
* @see #setParseStringAsXML(boolean)
* @return always returns false
* @deprecated This feature has been removed for security reasons (potential XML Entity Expansion attacks).
* This method now always returns false and will be removed in a future version.
*/
@Deprecated
public boolean getParseStringAsXML() {
return parseStringAsXML;
return false;
}
/**
* When set to true the StringAdapter will interpret its String value
* as containing XML text and parse it to a DOM Element. The new DOM
* Element will be a child of this String element. (i.e. wrapped in an
* element of the property name specified for this StringAdapter).
*
* @param parseStringAsXML when set to true the StringAdapter will interpret its String value as containing XML text
* @see #getParseStringAsXML()
* @param parseStringAsXML ignored
* @deprecated This feature has been removed for security reasons (potential XML Entity Expansion attacks).
* This method is now a no-op and will be removed in a future version.
*/
@Deprecated
public void setParseStringAsXML(boolean parseStringAsXML) {
this.parseStringAsXML = parseStringAsXML;
// no-op - feature removed for security reasons
}
}