This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 620fcbd15 WW-5621 Harden XML parsers against Entity Expansion (Billion
Laughs) attacks (#1642)
620fcbd15 is described below
commit 620fcbd152e436463d2ad8d6afbc93e518ff3452
Author: Lukasz Lenart <[email protected]>
AuthorDate: Sat Apr 4 11:20:33 2026 +0200
WW-5621 Harden XML parsers against Entity Expansion (Billion Laughs)
attacks (#1642)
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.
- Remove unused parseStringAsXML feature from StringAdapter to eliminate
a theoretical XML Entity Expansion vector
- Deprecate setParseStringAsXML() and getParseStringAsXML() for removal
- Enable SECURE_PROCESSING feature in DigesterDefinitionsReader
- Add unit test verifying JDK's entity expansion limit rejects
Billion Laughs payloads
- Add research document with vulnerability analysis
Co-authored-by: Claude <[email protected]>
---
.../java/org/apache/struts2/util/DomHelper.java | 2 +
.../org/apache/struts2/util/DomHelperTest.java | 38 ++++++-
.../digester/DigesterDefinitionsReader.java | 3 +
.../digester/TestDigesterDefinitionsReader.java | 23 ++++
.../apache/struts2/result/xslt/StringAdapter.java | 52 +++------
.../2026-01-13-billion-laughs-xxe-hardening.md | 116 +++++++++++++++++++++
6 files changed, 193 insertions(+), 41 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/util/DomHelper.java
b/core/src/main/java/org/apache/struts2/util/DomHelper.java
index 751da93c0..3f6554290 100644
--- a/core/src/main/java/org/apache/struts2/util/DomHelper.java
+++ b/core/src/main/java/org/apache/struts2/util/DomHelper.java
@@ -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);
}
diff --git a/core/src/test/java/org/apache/struts2/util/DomHelperTest.java
b/core/src/test/java/org/apache/struts2/util/DomHelperTest.java
index 68e6021c1..9c2d4181b 100644
--- a/core/src/test/java/org/apache/struts2/util/DomHelperTest.java
+++ b/core/src/test/java/org/apache/struts2/util/DomHelperTest.java
@@ -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);
+ }
+ }
}
diff --git
a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/digester/DigesterDefinitionsReader.java
b/plugins/tiles/src/main/java/org/apache/tiles/core/definition/digester/DigesterDefinitionsReader.java
index 6370b3703..47c076be4 100644
---
a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/digester/DigesterDefinitionsReader.java
+++
b/plugins/tiles/src/main/java/org/apache/tiles/core/definition/digester/DigesterDefinitionsReader.java
@@ -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);
}
diff --git
a/plugins/tiles/src/test/java/org/apache/tiles/core/definition/digester/TestDigesterDefinitionsReader.java
b/plugins/tiles/src/test/java/org/apache/tiles/core/definition/digester/TestDigesterDefinitionsReader.java
index f8a5ee0a3..54841462e 100644
---
a/plugins/tiles/src/test/java/org/apache/tiles/core/definition/digester/TestDigesterDefinitionsReader.java
+++
b/plugins/tiles/src/test/java/org/apache/tiles/core/definition/digester/TestDigesterDefinitionsReader.java
@@ -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)}.
*/
diff --git
a/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/StringAdapter.java
b/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/StringAdapter.java
index 4c401f436..0f3d648ea 100644
---
a/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/StringAdapter.java
+++
b/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/StringAdapter.java
@@ -18,13 +18,10 @@
*/
package org.apache.struts2.result.xslt;
-import org.apache.struts2.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(forRemoval = true, since = "7.2.0")
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(forRemoval = true, since = "7.2.0")
public void setParseStringAsXML(boolean parseStringAsXML) {
- this.parseStringAsXML = parseStringAsXML;
+ // no-op - feature removed for security reasons
}
}
diff --git
a/thoughts/shared/research/2026-01-13-billion-laughs-xxe-hardening.md
b/thoughts/shared/research/2026-01-13-billion-laughs-xxe-hardening.md
new file mode 100644
index 000000000..967bfe0aa
--- /dev/null
+++ b/thoughts/shared/research/2026-01-13-billion-laughs-xxe-hardening.md
@@ -0,0 +1,116 @@
+---
+date: 2026-01-13T00:00:00Z
+topic: "XML Entity Expansion (Billion Laughs) Hardening Analysis"
+tags: [research, security, xxe, billion-laughs, DomHelper, xml-parsing,
hardening]
+jira: WW-5621
+status: complete
+severity: Low (hardening only - not an exploitable vulnerability)
+---
+
+# Research: XML Entity Expansion (Billion Laughs) Hardening Analysis
+
+**Date**: 2026-01-13
+**Jira**: [WW-5621](https://issues.apache.org/jira/browse/WW-5621)
+
+## Research Question
+Assess the scope of a reported Billion Laughs DoS concern in Apache Struts XML
parsers and determine appropriate hardening measures.
+
+## Summary
+
+**NOT A VULNERABILITY**: While the SAX parser in `DomHelper.java` does not
explicitly block DOCTYPE declarations with internal entity expansion, modern
JDKs (7u45+) enforce a built-in 64,000 entity expansion limit that already
prevents Billion Laughs attacks. Additionally, none of the `DomHelper.parse()`
call sites accept user-supplied input — all XML sources come from the classpath.
+
+This analysis led to **defense-in-depth hardening** and removal of an
unnecessary feature that could theoretically become an attack surface if
misused by custom application code.
+
+## Why This Is Not a Vulnerability
+
+1. **JDK protection is already in place**: Since JDK 7u45, the XML parser
enforces a hard limit of 64,000 entity expansions. A Billion Laughs payload is
rejected with a `SAXParseException` before causing meaningful resource
consumption. This is verified by a unit test.
+
+2. **No user-controlled input reaches the parsers**: All callers of
`DomHelper.parse()` load XML exclusively from the classpath via
`ClassLoader.getResources()`:
+ - `XmlConfigurationProvider` — parses `struts.xml` and `<include>` files
+ - `DefaultValidatorFileParser` — parses `*-validation.xml` and
`*-validators.xml`
+
+3. **The StringAdapter path was disabled by default**: `parseStringAsXML`
defaulted to `false`, no subclasses of `StringAdapter` exist in the codebase,
and enabling it required custom Java code across three layers (custom adapter,
custom result, struts.xml registration).
+
+## Detailed Findings
+
+### XML Parser Configuration in DomHelper
+
+**File:** `core/src/main/java/org/apache/struts2/util/DomHelper.java`
+
+**Current protection:**
+```java
+factory.setFeature("http://xml.org/sax/features/external-general-entities";,
false);
+factory.setFeature("http://xml.org/sax/features/external-parameter-entities";,
false);
+```
+
+External entities are properly blocked. Internal entity expansion (used by
Billion Laughs) is handled by the JDK's built-in limit.
+
+### DomHelper.parse() Call Sites
+
+| Location | What is parsed | User input? |
+|----------|---------------|-------------|
+| `XmlConfigurationProvider.java:173` | struts.xml + includes from classpath |
No |
+| `DefaultValidatorFileParser.java:94` | Action validation files from
classpath | No |
+| `DefaultValidatorFileParser.java:131` | Validator definitions from classpath
| No |
+
+All sources are classpath resources. An attacker would need write access to
the application's classpath (WEB-INF/classes or deployed JARs) to inject a
malicious XML file — at which point they already have full control of the
application.
+
+### StringAdapter.parseStringAsXML (removed)
+
+**File:**
`plugins/xslt/src/main/java/org/apache/struts2/result/xslt/StringAdapter.java`
+
+This feature allowed a `StringAdapter` subclass to parse its string value as
XML via `DomHelper.parse()`. While disabled by default, it represented
unnecessary attack surface:
+- No subclasses of `StringAdapter` exist anywhere in the codebase
+- Enabling it required custom Java code, not just configuration
+- The XSLT plugin itself is niche
+
+**Decision**: Remove the feature entirely and deprecate the API methods for
future removal.
+
+### Tiles Plugin - DigesterDefinitionsReader
+
+**File:**
`plugins/tiles/src/main/java/org/apache/tiles/core/definition/digester/DigesterDefinitionsReader.java`
+
+Already had good protection (external entities and DTD loading disabled).
Added `FEATURE_SECURE_PROCESSING` as defense-in-depth. Only parses internal
Tiles definition files from the classpath.
+
+### XSLTResult TransformerFactory
+
+**File:**
`plugins/xslt/src/main/java/org/apache/struts2/result/xslt/XSLTResult.java`
+
+Already properly secured:
+```java
+factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+```
+
+No changes needed.
+
+## Hardening Measures Applied
+
+1. **Removed `parseStringAsXML` from StringAdapter** — eliminates a
theoretical attack surface that could be misused by custom application code
+2. **Deprecated `getParseStringAsXML()` and `setParseStringAsXML()`** — marked
for removal in a future version
+3. **Enabled `FEATURE_SECURE_PROCESSING` in DigesterDefinitionsReader** —
defense-in-depth
+4. **Added unit test** — verifies the JDK's 64K entity expansion limit rejects
Billion Laughs payloads, serving as a regression guard
+
+## Entity Expansion Impact (for reference)
+
+Without the JDK limit, a Billion Laughs payload would cause:
+
+| Level | Payload | Memory | Time |
+|-------|---------|--------|------|
+| 3 | ~500 bytes | 3 KB | 35 ms |
+| 5 | ~500 bytes | 300 KB | 91 ms |
+| 7 | ~500 bytes | 30 MB | 3408 ms, 1837 MB memory |
+
+The JDK limit stops expansion at 64,000 entities, well before these levels
become dangerous.
+
+## Code References
+
+- `core/src/main/java/org/apache/struts2/util/DomHelper.java` — XML parser
with external entity protection
+-
`plugins/xslt/src/main/java/org/apache/struts2/result/xslt/StringAdapter.java`
— removed parseStringAsXML feature
+-
`plugins/tiles/src/main/java/org/apache/tiles/core/definition/digester/DigesterDefinitionsReader.java`
— added SECURE_PROCESSING
+- `core/src/test/java/org/apache/struts2/util/DomHelperTest.java` — Billion
Laughs regression test
+
+## References
+
+- OWASP XXE Prevention Cheat Sheet:
https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
+- Billion Laughs Attack: https://en.wikipedia.org/wiki/Billion_laughs_attack
\ No newline at end of file
