mirror of
https://github.com/apache/struts.git
synced 2026-08-07 07:37:20 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
+3
@@ -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);
|
||||
}
|
||||
|
||||
+23
@@ -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)}.
|
||||
*/
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user