ppkarwasz commented on code in PR #22:
URL: https://github.com/apache/commons-xml/pull/22#discussion_r3552472038
##########
src/test/java/org/apache/commons/xml/BillionLaughsTest.java:
##########
@@ -24,237 +24,192 @@
/**
* Checks whether parsers reject a Billion Laughs payload (nested entity
expansion in the internal DTD subset).
*
- * <p>Two fixtures are used:</p>
+ * <p>Each {@code hardened*} test asserts the library blocks the payload; its
{@code unconfigured*} positive control asserts the same payload parses once the
+ * limit is disabled, so a block reflects the hardening rather than a broken
wrapper. The library pins no custom entity-expansion limit; each parser keeps
its own
+ * secure-processing default, which varies by implementation: {@code 2,500}
(stock JDK), {@code 64,000} (external Xerces under {@code
FEATURE_SECURE_PROCESSING}),
+ * {@code 100,000} (external Xerces' and Woodstox's own security managers). A
payload therefore has to exceed the largest of these.</p>
*
- * <ul>
- * <li>The <strong>medium</strong> fixture nests three levels of 16x
expansion ({@code lol1} through {@code lol3}), so resolving {@code &lol3;}
produces
- * {@code 16 * 16 * 16 = 4096} leaf {@code &lol;} expansions and a total
of {@code 1 + 16 + 256 + 4096 = 4369} entity-expansion events. That is
- * comfortably over the {@code entityExpansionLimit = 2500} this library
pins on every JAXP factory it returns (mirroring JDK 25's secure value), so the
- * hardened side trips on every JDK from 8 to 25. The unconfigured side
disables the limit explicitly via {@code setAttribute/setProperty} on the JDK
- * property name, so it parses the ~4 KB of expanded {@code "A"} text
and finishes immediately.</li>
- * <li>The <strong>large</strong> fixture nests seven levels of 10x
expansion ({@code lol1} through {@code lol7}), so resolving {@code &lol7;}
produces
- * {@code 10^7 = 10 000 000} leaf expansions, ~10 MB of {@code "A"}
text, and an amplification factor in the tens of thousands. Sized to trip
- * libexpat >= 2.4's built-in billion-laughs check (8 MiB activation
threshold and 100x amplification factor).</li>
- * </ul>
- *
- * <p>Why a single character {@code "A"}: XSLTC compiles a stylesheet's
expanded text into a JVM string constant, which is capped at 65535 bytes. A
larger
- * expansion makes {@code newTransformer(stylesheet)} fail with a misleading
"GregorSamsa" stub-class error even when entity limits are disabled. The medium
- * fixture is sized to stay well under that ceiling. The large fixture cannot
be used for {@code Templates} / {@code Transformer} compilation for the same
- * reason.</p>
- *
- * <p>Which fixture each test uses:</p>
+ * <p>Every payload shares one six-level x10 {@link #DTD} (declaring entities
is free until they are referenced) and varies only the body it expands:</p>
*
* <ul>
- * <li>{@code hardened*} tests pull from {@code hardened*Payload} helpers:
large fixture on Android (libexpat is the only defense), medium fixture on JDK
- * (entity-expansion count limit is sufficient).</li>
- * <li>{@code unconfigured*} positive controls pull from {@code
medium*Payload} helpers regardless of platform: libexpat's billion-laughs check
cannot be
- * disabled from Java, so a permissive parse of the large fixture would
still trip on Android.</li>
+ * <li>{@link #CONTENT_101K} ({@code 101000}) on the JVM: above every JVM
parser default.</li>
+ * <li>{@link #CONTENT_9M} ({@code 9000000}) on Android: above libexpat's 8
MiB billion-laughs activation threshold, the only defense there since the limit
is
+ * not configurable. For that same reason the positive controls do not
run on Android (see {@link #assumeSAXLimitConfigurable()}): a payload the
hardened test
+ * blocks cannot be parsed even without hardening.</li>
* </ul>
+ *
+ * <p>Why a single character {@code "A"}: it makes the expanded size equal the
expansion count, so a payload's size maps directly onto each parser's limit.</p>
*/
class BillionLaughsTest {
- private static final String LARGE_CONTENT = "&lol7;";
-
- private static final String LARGE_DTD =
- " <!ENTITY lol \"A\">\n"
- + " <!ENTITY lol1
\"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n"
- + " <!ENTITY lol2
\"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n"
- + " <!ENTITY lol3
\"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n"
- + " <!ENTITY lol4
\"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\n"
- + " <!ENTITY lol5
\"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\n"
- + " <!ENTITY lol6
\"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\n"
- + " <!ENTITY lol7
\"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\n";
-
- private static final String MEDIUM_CONTENT = "&lol3;";
-
- private static final String MEDIUM_DTD =
- " <!ENTITY lol \"A\">\n"
- + " <!ENTITY lol1
\"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n"
- + " <!ENTITY lol2
\"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n"
- + " <!ENTITY lol3
\"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n";
-
- /**
- * Hardened-side payload for DOM/SAX/XmlReader
- *
- * <ul>
- * <li>~10 MiB on Android to trip libexpats 8 MiB limit, and</li>
- * <li>~4 KiB on JDK to trip JDK 25's {@code entityExpansionLimit =
2500}.</li>
- * </ul>
- */
- private static String hardenedXmlPayload() {
- return AttackTestSupport.IS_ANDROID
- ? withDoctype("root", LARGE_DTD,
AttackTestSupport.xmlBody(LARGE_CONTENT))
- : mediumXmlPayload();
- }
-
+ /** 100,000 + 1,000 = 101,000 expansions; above every JVM parser's secure
default (2,500 / 64,000 / 100,000). */
+ private static final String CONTENT_101K = "&lol5;&lol3;";
+ /** 9 x 1,000,000 = 9,000,000 expansions; above libexpat's 8 MiB
billion-laughs activation threshold. */
+ private static final String CONTENT_9M = repeatRef("lol6", 9);
/**
- * Hardened-side XSD payload
- *
- * <ul>
- * <li>~10 MiB on Android to trip libexpats 8 MiB limit, and</li>
- * <li>~4 KiB on JDK to trip JDK 25's {@code entityExpansionLimit =
2500}.</li>
- * </ul>
+ * Shared DTD for every payload: a six-level x10 ladder, {@code &lol1;}
through {@code &lol6;} ({@code &lol6;} expands to 1,000,000). Declaring an
entity costs
+ * nothing until it is referenced, so the DTD is identical on every
platform and only the expanded body ({@link #content()}) varies.
*/
- private static String hardenedXsdPayload() {
- return AttackTestSupport.IS_ANDROID
- ? withDoctype("xs:schema", LARGE_DTD,
AttackTestSupport.xsdBody(LARGE_CONTENT))
- : mediumXsdPayload();
- }
+ private static final String DTD =
+ " <!ENTITY lol \"A\">\n"
+ + entityLine("lol1", "lol") // 10
+ + entityLine("lol2", "lol1") // 100
+ + entityLine("lol3", "lol2") // 1000
+ + entityLine("lol4", "lol3") // 10000
+ + entityLine("lol5", "lol4") // 100000
+ + entityLine("lol6", "lol5"); // 1000000
/**
- * Hardened-side XSLT payload
+ * Skips a positive control on Android.
*
- * <ul>
- * <li>~10 MiB on Android to trip libexpats 8 MiB limit, and</li>
- * <li>~4 KiB on JDK to trip JDK 25's {@code entityExpansionLimit =
2500}, and</li>
- * <li>stay under XSLTC's 60 KB constant-pool cap, and</li>
- * </ul>
+ * <p>The controls prove the hardened test blocked a payload that would
otherwise parse, so they must use the very payload the hardened test blocks.
+ * On Android the entity-expansion limit is not configurable (libexpat's
billion-laughs check cannot be lifted), so that payload
+ * cannot be parsed even without hardening, leaving nothing to prove.</p>
*/
- private static String hardenedXsltPayload() {
- return AttackTestSupport.IS_ANDROID
- ? withDoctype("xsl:stylesheet", LARGE_DTD,
AttackTestSupport.xsltBody(LARGE_CONTENT))
- : mediumXsltPayload();
+ private static void assumeSAXLimitConfigurable() {
+ Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on
Android: the entity-expansion limit is not configurable");
}
Review Comment:
Fixed in
https://github.com/apache/commons-xml/pull/22/commits/0676eb49dac3e8c872a13f7dd4df46e54de68e1b
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
