This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/max-nesting-depth in repository https://gitbox.apache.org/repos/asf/ws-xmlschema.git
commit 5a19b706463174ec055c5605d469443426c14166 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Wed Aug 26 10:05:29 2026 +0100 Add max nesting depth --- README.txt | 7 + THREAT-MODEL.md | 58 +++++--- .../apache/ws/commons/schema/SchemaBuilder.java | 122 +++++++++++++++- .../ws/commons/schema/XmlSchemaCollection.java | 14 ++ .../src/test/java/tests/NestingDepthLimitTest.java | 158 +++++++++++++++++++++ 5 files changed, 337 insertions(+), 22 deletions(-) diff --git a/README.txt b/README.txt index 218ce58b..f69ffb67 100644 --- a/README.txt +++ b/README.txt @@ -50,12 +50,19 @@ adjust the per-document limits: Maximum number of schema documents resolved during a single top-level read. The default is 1000. + org.apache.ws.commons.schema.maxNestingDepth + Maximum structural nesting depth while building the schema model, + including nested include/import/redefine document resolutions. The + default is 512. + For example, set a limit with: -Dorg.apache.ws.commons.schema.walker.maxDecisionPoints=20000 -Dorg.apache.ws.commons.schema.maxImportDepth=128 + -Dorg.apache.ws.commons.schema.maxNestingDepth=256 + =================== Support =================== diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 3a9be679..4fedfad5 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -29,7 +29,7 @@ at clone time. A report against project release *N* should be triaged against the model as it stood at *N*, not at HEAD. Latest release documented in `RELEASE-NOTE.txt`: 2.3.0. -- **Date**: 2026-05-30. +- **Date**: 2026-08-26. - **Authors**: ASF Security team, awaiting XMLSchema / Webservices PMC review. - **Status**: under maintainer review. @@ -47,7 +47,7 @@ *(inferred)* = synthesized by the producer from code structure or domain knowledge, awaiting PMC ratification (every *(inferred)* tag has a matching §14 question). -- **Model confidence**: 21 documented / 0 maintainer / 24 inferred. +- **Model confidence**: 22 documented / 0 maintainer / 24 inferred. XMLSchema is a Java library that parses, models, walks, and serializes W3C XML Schema documents (`.xsd` files). It is *not* a document @@ -215,11 +215,12 @@ A finding is in-model only if it reaches a row marked **yes**. - **Memory**: schemas are held in memory; XMLSchema has no built-in ceiling on schema-document size, imported bytes, or fetch rate. Import and include resolution is bounded per read by a default maximum depth of - 64 and maximum of 1000 resolved schema documents; both are configurable - with JVM system properties. The walker has active-path cycle detection - for the schema expansion graphs it traverses, but large acyclic schemas - may still consume substantial memory and CPU *(documented: - `README.txt`)*. + 64 and maximum of 1000 resolved schema documents, and schema-model + construction is bounded by a default structural nesting depth of 512; + these limits are configurable with JVM system properties. The walker has + active-path cycle detection for the schema expansion graphs it traverses, + but large acyclic schemas may still consume substantial memory and CPU + *(documented: `README.txt`)*. - **System properties**: `org.apache.ws.commons.schema.extension_registry` is consulted at `XmlSchemaCollection` construction time, and the named class is loaded via `Class.forName()` *(documented: @@ -262,6 +263,7 @@ points*: | `org.apache.ws.commons.schema.walker.maxReplayedEvents` system property | `1000000` *(documented: `XmlSchemaPathFinder.java`)* | operator-tunable per-process limit | maximum previously traversed events replayed while backtracking through one document | | `org.apache.ws.commons.schema.maxImportDepth` system property | `64` *(documented: `README.txt`)* | operator-tunable per-process limit | maximum import/include resolution depth for one schema read | | `org.apache.ws.commons.schema.maxSchemaResolutions` system property | `1000` *(documented: `README.txt`)* | operator-tunable per-process limit | maximum schema documents resolved during one top-level read | +| `org.apache.ws.commons.schema.maxNestingDepth` system property | `512` *(documented: `README.txt`)* | operator-tunable per-process limit | maximum structural nesting depth while building the schema model, including nested include/import/redefine document resolutions | | `DocumentBuilderFactory` provider | JDK default (typically Xerces fork) *(inferred — §14 Q6)* | depends on the JDK | shape of XML parsing for `read(InputSource)` / `read(InputStream)` paths | ### The insecure-default case @@ -313,6 +315,10 @@ feature is the intended defense and is sufficient) or `MODEL-GAP` default maximum of 1000 resolved schema documents per top-level read; both limits are configurable with JVM system properties *(documented: `README.txt`)*. +- Schema-model construction has a default maximum structural nesting depth + of 512, including nested include/import/redefine document resolutions; + the limit is configurable with a JVM system property *(documented: + `README.txt`)*. - Walker expansion cycles are rejected for type derivation, substitution groups, model groups, and attribute groups. This prevents recursive stack exhaustion for malformed but parseable schemas; it is not a general @@ -406,6 +412,18 @@ feature is the intended defense and is sufficient) or `MODEL-GAP` - **Severity**: **medium** availability impact when the embedding application accepts untrusted schemas *(inferred — §14 Q23)*. +### P6 — Bounded schema-model structural descent + +- **Condition**: attacker-controlled schema bytes or DOM content are passed + to `XmlSchemaCollection.read(...)`. +- **Property**: deeply nested schema structure, including depth split across + nested include/import/redefine document resolutions, terminates with + `XmlSchemaException` instead of exhausting the Java thread stack. +- **Violation symptom**: a parseable schema causes `SchemaBuilder` to recurse + until `StackOverflowError` or another resource-exhaustion failure. +- **Severity**: **medium** availability impact when the embedding + application accepts untrusted schemas. + ## §9 Security properties the project does *not* provide State each plainly so a triager can route an inbound report to the @@ -432,10 +450,10 @@ matching disclaimer. is moot — the caller's parser produced the DOM *(documented: `XmlSchemaCollection.read(Document)` / `.read(Element)`)*. - **No bound on schema-document size, imported bytes, or fetch rate.** - Import/include depth and total resolved documents per top-level read are - bounded, but a schema can still consume substantial resources within - those limits *(inferred — §14 Q8; depth and resolution limits documented - in `README.txt`)*. + Import/include depth, total resolved documents per top-level read, and + structural nesting depth are bounded, but a schema can still consume + substantial resources within those limits *(inferred — §14 Q8; resource + limits documented in `README.txt`)*. - **No protection of imported schemas at rest.** Schemas pulled from HTTP are fetched in cleartext if the URL is `http://`. The caller must use TLS-protected URLs or install a restricting resolver @@ -483,9 +501,10 @@ matching disclaimer. - **SSRF via `xs:import schemaLocation`** — see §9 first bullet. - **Billion-laughs / quadratic blowup** — partially mitigated by `FEATURE_SECURE_PROCESSING=true`, but not universally. -- **Schema-amplification DoS** — a deeply nested or heavily-recursive - acyclic schema can exhaust memory or CPU; cyclic walker expansion is - rejected as described in §8 P5. +- **Schema-amplification DoS** — large or heavily-recursive acyclic schemas + can exhaust memory or CPU within the documented resource limits; + structural nesting and cyclic walker expansion are rejected as described + in §8 P5 and §8 P6. - **Confused-deputy fetch via untrusted `baseUri` + relative `schemaLocation`** — the operator-supplied base URI is trusted. @@ -509,8 +528,8 @@ The embedding Java application **must**: *(inferred — §14 Q6)*. 4. Bound maximum schema size, imported bytes, and fetch rate at the *caller* level. XMLSchema provides configurable import/include depth - and per-read resolution limits, but these do not replace deployment- - specific resource budgets *(inferred — §14 Q8)*. + per-read resolution, and structural nesting limits, but these do not + replace deployment-specific resource budgets *(inferred — §14 Q8)*. 5. Set `org.apache.ws.commons.schema.extension_registry` only at JVM startup from a trusted source; do not allow untrusted actors to set it. @@ -529,9 +548,9 @@ defense-in-depth controls: `FEATURE_SECURE_PROCESSING=true`: `disallow-doctype-decl=true`, `external-general-entities=false`, and `external-parameter-entities=false`. -3. Supplement XMLSchema's import/include depth and per-read resolution - limits with caller-boundary budgets for total imported bytes and fetch - rate per top-level parse. +3. Supplement XMLSchema's import/include depth, per-read resolution, and + structural nesting limits with caller-boundary budgets for total imported + bytes and fetch rate per top-level parse. 4. Use connect/read timeouts for import fetches and fail closed on timeout or policy-check errors. 5. Log import-resolution decisions (requested URI, normalized target, @@ -817,6 +836,7 @@ source comments. The project website is | `README.txt` | "lightweight schema object model that can be used to manipulate and generate XML schema representations" | §1, §2 intended use | | `RELEASE-NOTE.txt` (2.3.0) | Java 17 minimum, Java 7 dropped | §5 environment | | `xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java` line 361 | `org.apache.ws.commons.schema.extension_registry` system property loaded via `Class.forName` | §5a, §6, §11 | +| `xmlschema-core/src/main/java/org/apache/ws/commons/schema/SchemaBuilder.java` | `org.apache.ws.commons.schema.maxNestingDepth` structural descent limit | §5, §5a, §6, §8 P6 | | `XmlSchemaCollection.java` line 713 | `docFac.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, TRUE)` | §5a, §8 P2 | | `XmlSchemaCollection.java` line 745 | `AccessController.doPrivileged` wrapper for the SAX parse | §5 | | `XmlSchema.java` line 886 | `trFac.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, TRUE)` for serializer | §5a, §8 P2 | diff --git a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/SchemaBuilder.java b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/SchemaBuilder.java index 92b366af..66418f23 100644 --- a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/SchemaBuilder.java +++ b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/SchemaBuilder.java @@ -69,6 +69,22 @@ public class SchemaBuilder { private static final String[] RESERVED_ATTRIBUTES_LIST = { "name", "type", "default", "fixed", "form", "id", "use", "ref" }; + /** + * Maximum nesting depth of the structural descent. The DOM-to-model walk + * recurses once per nesting level of the schema document, so a bound is + * required to keep a deeply nested document from overflowing the thread + * stack (neither FEATURE_SECURE_PROCESSING nor JDK defaults impose an + * element-depth limit on the XML parse itself). The counter lives on the + * XmlSchemaCollection, not on this builder: XmlSchemaCollection.read + * creates a fresh SchemaBuilder for every include/import/redefine + * document that resolveXmlSchema pulls in, but all those documents are + * built on one thread stack, so the depth must be carried across the + * hops rather than reset per document. Each nested document resolution + * counts one level toward the same bound (see resolveXmlSchema), which + * also bounds a chain of distinct single-hop documents. + */ + private static final int MAX_NESTING_DEPTH = + getIntProperty("org.apache.ws.commons.schema.maxNestingDepth", 512); XmlSchemaCollection collection; Document currentDocument; XmlSchema currentSchema; @@ -115,6 +131,38 @@ public class SchemaBuilder { } } + private void enterNestedStructure() { + // Check before incrementing: on a breach the counter is left + // untouched, so the shared per-collection count stays balanced (every + // completed increment has a matching finally-decrement) even when the + // exception is caught and the collection is reused. + if (collection.builderNestingDepth >= MAX_NESTING_DEPTH) { + throw new XmlSchemaException("The schema is nested more than " + MAX_NESTING_DEPTH + + " levels deep (nested include/import/redefine document" + + " resolutions count toward the same depth bound); refusing to" + + " build it. The limit may be changed with the" + + " org.apache.ws.commons.schema.maxNestingDepth" + + " system property."); + } + collection.builderNestingDepth++; + } + + private void exitNestedStructure() { + collection.builderNestingDepth--; + } + + private static int getIntProperty(String name, int defaultValue) { + try { + Integer value = Integer.getInteger(name); + if (value != null) { + return value.intValue(); + } + } catch (RuntimeException e) { + // fall through to the default + } + return defaultValue; + } + /** * Setup the cache to be used by the current thread of execution. Multiple threads can use the cache, and * each one must call this method at some point prior to attempting to resolve the first schema, or the @@ -275,6 +323,16 @@ public class SchemaBuilder { */ XmlSchemaComplexType handleComplexType(XmlSchema schema, Element complexEl, Element schemaEl, boolean topLevel) { + enterNestedStructure(); + try { + return doHandleComplexType(schema, complexEl, schemaEl, topLevel); + } finally { + exitNestedStructure(); + } + } + + private XmlSchemaComplexType doHandleComplexType(XmlSchema schema, Element complexEl, Element schemaEl, + boolean topLevel) { XmlSchemaComplexType ct = new XmlSchemaComplexType(schema, topLevel); @@ -381,6 +439,16 @@ public class SchemaBuilder { * @param isGlobal */ XmlSchemaElement handleElement(XmlSchema schema, Element el, Element schemaEl, boolean isGlobal) { + enterNestedStructure(); + try { + return doHandleElement(schema, el, schemaEl, isGlobal); + } finally { + exitNestedStructure(); + } + } + + private XmlSchemaElement doHandleElement(XmlSchema schema, Element el, Element schemaEl, + boolean isGlobal) { XmlSchemaElement element = new XmlSchemaElement(schema, isGlobal); @@ -587,6 +655,16 @@ public class SchemaBuilder { */ XmlSchemaSimpleType handleSimpleType(XmlSchema schema, Element simpleEl, Element schemaEl, boolean topLevel) { + enterNestedStructure(); + try { + return doHandleSimpleType(schema, simpleEl, schemaEl, topLevel); + } finally { + exitNestedStructure(); + } + } + + private XmlSchemaSimpleType doHandleSimpleType(XmlSchema schema, Element simpleEl, Element schemaEl, + boolean topLevel) { XmlSchemaSimpleType simpleType = new XmlSchemaSimpleType(schema, topLevel); if (simpleEl.hasAttribute("name")) { simpleType.setName(simpleEl.getAttribute("name")); @@ -702,9 +780,20 @@ public class SchemaBuilder { if (collection.check(key)) { collection.push(key); try { - XmlSchema readSchema = collection.read(source, validator); - putCachedSchema(targetNamespace, schemaLocation, baseUri, readSchema); - return readSchema; + // The nested document is built by a fresh SchemaBuilder + // (collection.read creates one per document) but on this + // same thread stack, so the hop counts one level toward + // the shared per-collection nesting bound. Exact-key + // cycle detection (check/push) alone does not bound a + // chain of distinct schema locations. + enterNestedStructure(); + try { + XmlSchema readSchema = collection.read(source, validator); + putCachedSchema(targetNamespace, schemaLocation, baseUri, readSchema); + return readSchema; + } finally { + exitNestedStructure(); + } } finally { collection.pop(); } @@ -854,6 +943,15 @@ public class SchemaBuilder { } private XmlSchemaAll handleAll(XmlSchema schema, Element allEl, Element schemaEl) { + enterNestedStructure(); + try { + return doHandleAll(schema, allEl, schemaEl); + } finally { + exitNestedStructure(); + } + } + + private XmlSchemaAll doHandleAll(XmlSchema schema, Element allEl, Element schemaEl) { XmlSchemaAll all = new XmlSchemaAll(); @@ -1100,6 +1198,15 @@ public class SchemaBuilder { } private XmlSchemaChoice handleChoice(XmlSchema schema, Element choiceEl, Element schemaEl) { + enterNestedStructure(); + try { + return doHandleChoice(schema, choiceEl, schemaEl); + } finally { + exitNestedStructure(); + } + } + + private XmlSchemaChoice doHandleChoice(XmlSchema schema, Element choiceEl, Element schemaEl) { XmlSchemaChoice choice = new XmlSchemaChoice(); if (choiceEl.hasAttribute("id")) { @@ -1557,6 +1664,15 @@ public class SchemaBuilder { } private XmlSchemaSequence handleSequence(XmlSchema schema, Element sequenceEl, Element schemaEl) { + enterNestedStructure(); + try { + return doHandleSequence(schema, sequenceEl, schemaEl); + } finally { + exitNestedStructure(); + } + } + + private XmlSchemaSequence doHandleSequence(XmlSchema schema, Element sequenceEl, Element schemaEl) { XmlSchemaSequence sequence = new XmlSchemaSequence(); 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 0e5b32aa..dfe59fb3 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 @@ -198,6 +198,20 @@ public final class XmlSchemaCollection { return !stack.contains(pKey); } + /** + * Structural nesting depth of the schema build currently in progress on + * this collection. Maintained by SchemaBuilder (balanced increments and + * finally-decrements, so it always returns to zero when the outermost + * read completes or throws). It lives here rather than on SchemaBuilder + * because read() creates a fresh SchemaBuilder for every + * include/import/redefine document that SchemaBuilder.resolveXmlSchema + * pulls in, while all those documents are built on a single thread + * stack: the bound has to be carried across the nested reads, not reset + * per document. Package-private and accessed directly by SchemaBuilder, + * like {@link #baseUri}. + */ + int builderNestingDepth; + public ExtensionRegistry getExtReg() { return extReg; } diff --git a/xmlschema-core/src/test/java/tests/NestingDepthLimitTest.java b/xmlschema-core/src/test/java/tests/NestingDepthLimitTest.java new file mode 100644 index 00000000..407dafc9 --- /dev/null +++ b/xmlschema-core/src/test/java/tests/NestingDepthLimitTest.java @@ -0,0 +1,158 @@ +/** + * 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.File; +import java.io.FileWriter; +import java.io.StringReader; +import java.io.Writer; + +import org.apache.ws.commons.schema.XmlSchema; +import org.apache.ws.commons.schema.XmlSchemaCollection; +import org.apache.ws.commons.schema.XmlSchemaException; + +import org.junit.Assert; +import org.junit.Test; + +import org.xml.sax.InputSource; + +/** + * The structural descent of the schema build must be bounded: a deeply + * nested schema document must fail with the documented XmlSchemaException + * instead of exhausting the thread stack. The bound is shared across the + * documents of one read: every nested include/import/redefine resolution is + * built by a fresh SchemaBuilder but on the same thread stack, so the depth + * carries across resolveXmlSchema hops instead of resetting per document, + * and a chain of distinct single-hop documents is bounded too. + */ +public class NestingDepthLimitTest extends Assert { + @Test + public void testReasonablyNestedSchemaStillParses() throws Exception { + XmlSchemaCollection collection = new XmlSchemaCollection(); + XmlSchema schema = collection.read(new StringReader(buildNestedSchema(50))); + assertNotNull(schema); + } + + @Test + public void testDeeplyNestedSchemaIsRejected() throws Exception { + XmlSchemaCollection collection = new XmlSchemaCollection(); + try { + collection.read(new StringReader(buildNestedSchema(600))); + fail("A schema document nested 600 particle levels deep should be rejected."); + } catch (XmlSchemaException expected) { + assertTrue(expected.getMessage().contains("nested")); + } + } + + /** + * The counter must not reset across include hops: a chain of distinct + * documents, each individually trivial, must still hit the shared bound + * instead of adding stack frames per hop until the thread stack + * overflows. (Exact-key cycle detection never fires here because every + * schemaLocation is distinct.) + */ + @Test + public void testIncludeChainOfDistinctDocumentsIsRejected() throws Exception { + File root = createIncludeChain(newTempDir(), 600, 0); + XmlSchemaCollection collection = new XmlSchemaCollection(); + try { + collection.read(new InputSource(root.toURI().toString())); + fail("An include chain of 600 distinct documents should be rejected, not recursed."); + } catch (XmlSchemaException expected) { + assertTrue(expected.getMessage().contains("depth")); + } + } + + @Test + public void testShallowIncludeChainWithNestedLeafStillParses() throws Exception { + File root = createIncludeChain(newTempDir(), 3, 50); + XmlSchemaCollection collection = new XmlSchemaCollection(); + XmlSchema schema = collection.read(new InputSource(root.toURI().toString())); + assertNotNull(schema); + } + + @Test + public void testCollectionIsReusableAfterRejection() throws Exception { + XmlSchemaCollection collection = new XmlSchemaCollection(); + try { + collection.read(new StringReader(buildNestedSchema(600))); + fail("A schema document nested 600 particle levels deep should be rejected."); + } catch (XmlSchemaException expected) { + // expected; the counter must have unwound to zero. + } + + XmlSchema schema = collection.read(new StringReader(buildNestedSchema(50, "urn:after-rejection"))); + assertNotNull(schema); + } + + private String buildNestedSchema(int depth) { + return buildNestedSchema(depth, "urn:nesting"); + } + + private String buildNestedSchema(int depth, String namespace) { + StringBuilder schema = new StringBuilder(); + schema.append("<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\"";) + .append(" targetNamespace=\"").append(namespace).append("\">"); + schema.append("<xs:element name=\"root\">"); + for (int i = 0; i < depth; i++) { + schema.append("<xs:complexType><xs:sequence><xs:element name=\"e\">"); + } + schema.append("<xs:complexType/>"); + for (int i = 0; i < depth; i++) { + schema.append("</xs:element></xs:sequence></xs:complexType>"); + } + schema.append("</xs:element></xs:schema>"); + return schema.toString(); + } + + private File newTempDir() { + File dir = new File(System.getProperty("java.io.tmpdir"), + "xmlschema-nesting-" + System.nanoTime()); + assertTrue(dir.mkdir()); + return dir; + } + + /** + * Writes an include chain of the given length; every document is + * structurally trivial, and the last one carries the given nested + * payload (or none). + */ + private File createIncludeChain(File dir, int hops, int leafNestingDepth) throws Exception { + for (int i = 0; i < hops; i++) { + File f = new File(dir, "hop" + i + ".xsd"); + Writer out = new FileWriter(f); + try { + out.write("<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\""; + + " targetNamespace=\"urn:nesting\">"); + if (i + 1 < hops) { + out.write("<xs:include schemaLocation=\"hop" + (i + 1) + ".xsd\"/>"); + } else if (leafNestingDepth > 0) { + String leaf = buildNestedSchema(leafNestingDepth); + int start = leaf.indexOf("<xs:element"); + out.write(leaf.substring(start, leaf.lastIndexOf("</xs:schema>"))); + } + out.write("</xs:schema>"); + } finally { + out.close(); + } + } + return new File(dir, "hop0.xsd"); + } +}
