This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/entity-expansion in repository https://gitbox.apache.org/repos/asf/ws-xmlschema.git
commit e6c6b4e5d7a9df56d9dd8ea8bc0e5e3896c6c048 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Fri Sep 25 11:05:53 2026 +0100 Limit entity expansion --- README.txt | 11 ++- SECURITY.md | 7 +- THREAT-MODEL.md | 34 +++++-- .../ws/commons/schema/XmlSchemaCollection.java | 80 ++++++++++++--- .../test/java/tests/EntityExpansionLimitTest.java | 107 +++++++++++++++++++++ 5 files changed, 209 insertions(+), 30 deletions(-) diff --git a/README.txt b/README.txt index f075a5ff..a1929f61 100644 --- a/README.txt +++ b/README.txt @@ -76,14 +76,19 @@ stack. The following JVM system property adjusts the limit: The DOCTYPE declaration itself is accepted: an internal DTD subset is a legitimate part of many real schema documents - the W3C's own XML Signature, XML Encryption and XKMS schemas declare their target namespace - as an entity in one - and FEATURE_SECURE_PROCESSING bounds entity - expansion by both count and accumulated size. + as an entity in one - and entity expansion is bounded. + FEATURE_SECURE_PROCESSING bounds its accumulated size, and the parser + allows at most 1000 entity expansions per document, since the JDK expands + nested entities recursively and a long chain of them would otherwise + exhaust the thread stack. A lower jdk.xml.entityExpansionLimit is kept, + subject to the same jaxp.properties caveat as jdk.xml.maxElementDepth + below; raising it has no effect. The same parser also bounds the element depth of the document, including markup an entity expands to, so deep markup is refused before it can exhaust the thread stack. The limit is set by maxNestingDepth above: twice its value plus 64, which is 1088 by default, since the schema build - never accepts a document deeper than that. A lower jdk.xml.maxElementDepth, + never accepts a document deeper than that. A lower jdk.xml.maxElementDepth is kept, except that on JDK 8, 11 and 17 only a system property is seen: a lower limit set just in the JDK's jaxp.properties file is raised to this one there, so set -Djdk.xml.maxElementDepth instead. Raising diff --git a/SECURITY.md b/SECURITY.md index de700701..407570bd 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -59,9 +59,10 @@ source must install a restricting resolver via See [THREAT-MODEL.md](./THREAT-MODEL.md) section 10 for the full set of downstream responsibilities. -The parser `XmlSchemaCollection` reads schema documents with never -resolves external DTDs or external entities, and bounds the element depth -of what it parses, including markup that an entity expands to. That depth +The parser that `XmlSchemaCollection` reads schema documents with never +resolves external DTDs or external entities. It allows at most 1000 entity +expansions per document, and bounds the element depth of what it parses, +including markup that an entity expands to. That depth limit is set by `org.apache.ws.commons.schema.maxNestingDepth`: twice its value plus 64, which is 1088 by default. A lower `jdk.xml.maxElementDepth` is kept (on JDK 8, 11 and 17, only when set as a system property rather diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 551b4e6d..6dc2d868 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -93,7 +93,7 @@ filesystem IO** when it follows `<xs:include>` / `<xs:import>` / | **`ExtensionRegistry` implementation** | trusted | Pluggable via system property `org.apache.ws.commons.schema.extension_registry` *(documented: `xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java` line 361)*. | | **Producer of the schema bytes** (`Reader`, `InputStream`, `InputSource`, `Source`, `Document`, `Element`) | **variable** — see §6 trust table | The *only* attacker-controllable input position; in many embeddings the schema bytes come from a WSDL fetched off the wire. | | **Producer of imported / included schemas** (resolved by the `URIResolver`) | **variable** — typically as untrusted as the parent schema, but can be a *different* origin if the parent's `<xs:import schemaLocation="http://attacker/evil.xsd";>` points elsewhere | Following an `xs:import` is a **second, possibly cross-origin, fetch**. This is the principal SSRF surface. | -| **JDK XML platform** (`DocumentBuilderFactory`, `TransformerFactory`, `SchemaFactory`) | trusted upstream | XMLSchema sets `FEATURE_SECURE_PROCESSING=true` on the factories it constructs. Its internal schema parser also disables external DTD and external entity resolution. DOCTYPE declarations are accepted (an internal DTD subset is legitimate in real schema documents, and FSP bounds entity expansion by count and accumulated size). | +| **JDK XML platform** (`DocumentBuilderFactory`, `TransformerFactory`, `SchemaFactory`) | trusted upstream | XMLSchema sets `FEATURE_SECURE_PROCESSING=true` on the factories it constructs. Its internal schema parser also disables external DTD and external entity resolution. DOCTYPE declarations are accepted (an internal DTD subset is legitimate in real schema documents; FSP bounds the accumulated size of entity expansion, and the parser caps their number at 1000). | ### Component-family table @@ -169,8 +169,10 @@ A finding is in-model only if it reaches a row marked **yes**. entities, external parameter entities, and external DTD loading, and installs a no-op SAX `EntityResolver`. A DOCTYPE declaration is accepted: an internal DTD subset carries no external reference, and - entity expansion stays bounded by `FEATURE_SECURE_PROCESSING` on both - count and accumulated size. The parse is bounded in element depth too, + entity expansion stays bounded, in accumulated size by + `FEATURE_SECURE_PROCESSING` and in number at 1000 per document, low + enough that a chain of nested entities cannot exhaust the thread stack + (§8 P2). The parse is bounded in element depth too, expanded entity markup included, at twice `maxNestingDepth` plus 64 (§5a, §8 P6). There is no property to relax the external-resolution controls, and none to tighten the DOCTYPE posture. @@ -417,9 +419,11 @@ leave open. - **Property**: external general entities, external parameter entities, external DTD loading, and JAXP external-DTD access are disabled; a no-op `EntityResolver` is installed as a fallback. DOCTYPE declarations - are accepted, with internal entity expansion bounded by - `FEATURE_SECURE_PROCESSING` on both expansion count - (`JAXP00010001`) and accumulated entity size (`JAXP00010004`). + are accepted, with internal entity expansion bounded in number at 1000 + per document, or a lower `jdk.xml.entityExpansionLimit` (`JAXP00010001`), + and in accumulated size by `FEATURE_SECURE_PROCESSING` (`JAXP00010004`). + The JDK expands nested entities recursively, so the count also bounds + how deep a chain of entities can nest. - **Violation symptom**: attacker-controlled schema bytes cause the internal parser to fetch external DTD/entity content, or to expand entities past those limits. @@ -574,8 +578,10 @@ matching disclaimer. `read(Element)`. XMLSchema's internal parser path disables external DTD/entity resolution. - **SSRF via `xs:import schemaLocation`** — see §9 first bullet. -- **Billion-laughs / quadratic blowup** — partially mitigated by - `FEATURE_SECURE_PROCESSING=true`, but not universally. +- **Billion-laughs / quadratic blowup** — partially mitigated on the + internal parser path by `FEATURE_SECURE_PROCESSING=true` and the + 1000-expansion cap (§8 P2), but not universally: a caller that parses + the DOM it hands to `read(Document)` sets its own limits. - **Schema-amplification DoS** — large or heavily-recursive acyclic schemas can exhaust memory or CPU within the documented resource limits; structural nesting and cyclic or over-deep walker expansion are rejected @@ -777,6 +783,18 @@ Revise this document when any of the following lands: rule as first written: it tested only the URI authority, so `file:////host/share/x.xsd`, which parses with no authority and carries the host in its path instead, was not caught. +- **2026-09-25** — a change to an existing resource limit, a revision + trigger under the fourth bullet above: the internal parser now allows at + most 1000 entity expansions per document, keeping a lower + `jdk.xml.entityExpansionLimit`. The JDK's own limit of 64,000 (2,500 on + JDK 25) allowed a chain of internal entities, each referring to the next, + deep enough to overflow the thread stack inside the JDK parser, which + unwinds nested entities recursively: about 30,000 at the default stack + size on JDK 8 to 21, and 2,000 on a 256 KB stack on every JDK. A chain + adding text at each level ran out of memory instead. The parser also no + longer defers DOM node expansion, whose cost grew with the square of the + number of entities declared. §1, §4 reachability, §8 P2 and §9 are + updated. - **2026-09-25** — a new resource limit, a revision trigger under the fourth bullet above: the internal parser now bounds the element depth of the document it parses, at twice `maxNestingDepth` plus 64 (#179). A diff --git a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java index 7139bd4a..fe8adb2e 100644 --- a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java +++ b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java @@ -875,6 +875,11 @@ public final class XmlSchemaCollection { docFac.setNamespaceAware(true); hardenAgainstDtdProcessing(docFac); limitElementDepth(docFac); + limitEntityExpansion(docFac); + // The deferred DOM saves nothing when the whole document is built into a schema, and + // its time grows with the square of the number of entities an internal DTD subset + // declares: 60,000 declarations, 1.5 MB, took 20 seconds with it and 0.25 without. + trySetFeature(docFac, "http://apache.org/xml/features/dom/defer-node-expansion";, false); final DocumentBuilder builder = docFac.newDocumentBuilder(); builder.setEntityResolver(NO_OP_ENTITY_RESOLVER); Document doc = null; @@ -894,6 +899,12 @@ public final class XmlSchemaCollection { + " property plus 64, unless jdk.xml.maxElementDepth" + " sets a lower one.", e); } + if (e.getMessage() != null && e.getMessage().startsWith(ENTITY_EXPANSION_ERROR + ":")) { + throw new XmlSchemaException("The schema document expands too many entities to parse (" + + e.getMessage() + "). The limit is " + + MAX_ENTITY_EXPANSIONS + ", unless" + + " jdk.xml.entityExpansionLimit sets a lower one.", e); + } throw new XmlSchemaException(e.getMessage(), e); } } @@ -953,16 +964,52 @@ public final class XmlSchemaCollection { * deeper than about twice its structural bound, maxNestingDepth, since each counted level (an * element, a type, a model group) is at most two XML levels deep, so refusing deeper markup * in the parser costs nothing. A lower limit already set, by jdk.xml.maxElementDepth or by the - * JDK's own default, is left in place where it can be seen: see currentElementDepthLimit. + * JDK's own default, is left in place where it can be seen: see currentLimit. */ private static void limitElementDepth(DocumentBuilderFactory docFac) { - final long limit = elementDepthLimit(SchemaBuilder.MAX_NESTING_DEPTH); - final long existing = currentElementDepthLimit(docFac); + keepLowerLimit(docFac, MAX_ELEMENT_DEPTH, "jdk.xml.maxElementDepth", + elementDepthLimit(SchemaBuilder.MAX_NESTING_DEPTH)); + } + + /** + * The JDK parser's entity-expansion limit, <code>jdk.xml.entityExpansionLimit</code>, under + * the name JDK 8 onwards accepts on a factory. + */ + private static final String ENTITY_EXPANSION_LIMIT = + "http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit";; + + /** The code that starts the JDK parser's message when that limit is exceeded. */ + private static final String ENTITY_EXPANSION_ERROR = "JAXP00010001"; + + /** The most entity expansions one schema document may make. */ + static final long MAX_ENTITY_EXPANSIONS = 1000; + + /** + * Bound the number of entity expansions in the parse. The JDK unwinds nested entities + * recursively, one level of its own stack per level of nesting, so a chain of internal + * entities each referring to the next overflowed the thread stack on JDK 8 to 21: the + * default limit of 64,000 expansions (2,500 on JDK 25) allows a chain far deeper than a + * thread stack holds. A chain that also adds text at each level ran out of memory instead. + * Schema documents that use an internal DTD subset declare a few entities for namespace + * names and make a handful of references to them, so 1,000 costs nothing. A lower limit + * already set, by jdk.xml.entityExpansionLimit, is left in place where it can be seen. + */ + private static void limitEntityExpansion(DocumentBuilderFactory docFac) { + keepLowerLimit(docFac, ENTITY_EXPANSION_LIMIT, "jdk.xml.entityExpansionLimit", + MAX_ENTITY_EXPANSIONS); + } + + /** + * Set a JDK parser limit on the factory, unless a lower one is already in force. + */ + private static void keepLowerLimit(DocumentBuilderFactory docFac, String attribute, + String property, long limit) { + final long existing = currentLimit(docFac, attribute, property); if (existing > 0 && existing <= limit) { return; } try { - docFac.setAttribute(MAX_ELEMENT_DEPTH, String.valueOf(limit)); + docFac.setAttribute(attribute, String.valueOf(limit)); } catch (IllegalArgumentException e) { // A parser other than the JDK's, which does not recognize the property. } @@ -978,17 +1025,18 @@ public final class XmlSchemaCollection { } /** - * The element-depth limit already in force, or 0 for none. Only some JDKs report it through - * the factory: JDK 21 does; JDK 8 and 11 throw IllegalArgumentException, and JDK 17 reports - * only attributes set on the factory itself, returning null or throwing NullPointerException. - * When the factory gives no value, the system property the parser takes it from is read - * instead. On those JDKs a limit set only in the JDK's jaxp.properties file is not seen, and - * is raised to this library's own. + * The limit already in force for a JDK parser property, or 0 for none. Only some JDKs report + * it through the factory: JDK 21 does; JDK 8 and 11 throw IllegalArgumentException, and JDK 17 + * reports only attributes set on the factory itself, returning null or throwing + * NullPointerException. When the factory gives no value, the system property the parser takes + * it from is read instead. On those JDKs a limit set only in the JDK's jaxp.properties file is + * not seen, and is raised to this library's own. */ - private static long currentElementDepthLimit(DocumentBuilderFactory docFac) { + private static long currentLimit(DocumentBuilderFactory docFac, String attribute, + String property) { Object current = null; try { - current = docFac.getAttribute(MAX_ELEMENT_DEPTH); + current = docFac.getAttribute(attribute); } catch (RuntimeException e) { // Not reported by this JDK's factory. } @@ -999,7 +1047,7 @@ public final class XmlSchemaCollection { // Fall back to the system property. } } - return getIntProperty("jdk.xml.maxElementDepth", 0); + return getIntProperty(property, 0); } /** @@ -1013,9 +1061,9 @@ public final class XmlSchemaCollection { * normative schemas (XML Signature, XML Encryption, XKMS) declare the * entities they use for their target namespace in one - and refusing it * closes no attack path that is still open here: it holds no external - * reference, and {@code FEATURE_SECURE_PROCESSING} bounds entity expansion - * by both count and accumulated size, so neither nested nor flat expansion - * runs away. + * reference, {@code FEATURE_SECURE_PROCESSING} bounds the accumulated size of + * entity expansion, and limitEntityExpansion bounds their number tightly + * enough that a chain of nested entities cannot exhaust the thread stack. * </p> */ private static void hardenAgainstDtdProcessing(DocumentBuilderFactory docFac) { diff --git a/xmlschema-core/src/test/java/tests/EntityExpansionLimitTest.java b/xmlschema-core/src/test/java/tests/EntityExpansionLimitTest.java new file mode 100644 index 00000000..5fa7c146 --- /dev/null +++ b/xmlschema-core/src/test/java/tests/EntityExpansionLimitTest.java @@ -0,0 +1,107 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package tests; + +import java.io.StringReader; + +import org.apache.ws.commons.schema.XmlSchemaCollection; +import org.apache.ws.commons.schema.XmlSchemaException; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +/** + * The JDK parser unwinds nested entities recursively, so a chain of internal entities each + * referring to the next must be refused by the entity-expansion limit before it can exhaust the + * thread stack. + */ +public class EntityExpansionLimitTest extends Assert { + private static final String ENTITY_EXPANSION_LIMIT = "jdk.xml.entityExpansionLimit"; + + private String savedLimit; + + @Before + public void saveLimit() { + savedLimit = System.getProperty(ENTITY_EXPANSION_LIMIT); + System.clearProperty(ENTITY_EXPANSION_LIMIT); + } + + @After + public void restoreLimit() { + if (savedLimit == null) { + System.clearProperty(ENTITY_EXPANSION_LIMIT); + } else { + System.setProperty(ENTITY_EXPANSION_LIMIT, savedLimit); + } + } + + /** + * On a 256 KB stack a chain of 3,000 overflowed inside the JDK parser on JDK 8 to 25; at the + * default stack size a chain of about 30,000 did. + */ + @Test + public void testEntityChainIsRefusedOnASmallStack() throws Exception { + final Throwable[] thrown = new Throwable[1]; + Thread reader = new Thread(null, new Runnable() { + public void run() { + try { + new XmlSchemaCollection().read(new StringReader(chainedSchema(3000))); + } catch (Throwable t) { + thrown[0] = t; + } + } + }, "entity-chain", 256 * 1024); + reader.start(); + reader.join(); + assertTrue("Expected XmlSchemaException, got " + thrown[0], + thrown[0] instanceof XmlSchemaException); + assertTrue(thrown[0].getMessage(), thrown[0].getMessage().contains("too many entities")); + } + + @Test + public void testEntityChainWithinTheLimitStillParses() { + assertNotNull(new XmlSchemaCollection().read(new StringReader(chainedSchema(900)))); + } + + @Test + public void testLowerEntityExpansionLimitIsKept() { + System.setProperty(ENTITY_EXPANSION_LIMIT, "5"); + try { + new XmlSchemaCollection().read(new StringReader(chainedSchema(10))); + fail("Ten expansions should be refused under a limit of five."); + } catch (XmlSchemaException expected) { + assertTrue(expected.getMessage(), expected.getMessage().contains("too many entities")); + } + } + + private static String chainedSchema(int length) { + StringBuilder schema = new StringBuilder("<!DOCTYPE xs:schema ["); + for (int i = 1; i < length; i++) { + schema.append("<!ENTITY e").append(i).append(" '&e").append(i + 1).append(";'>"); + } + schema.append("<!ENTITY e").append(length).append(" 'x'>]>") + .append("<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\";>") + .append("<xs:annotation><xs:documentation>&e1;</xs:documentation></xs:annotation>") + .append("</xs:schema>"); + return schema.toString(); + } +}
