From d9a6214e5f6772ac9e4bf1a65643459c91a73e9c Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Tue, 1 Sep 2026 11:53:06 +0200 Subject: [PATCH 1/2] Bound the content model a schema expands into A schema loader expands a repeated particle into content-model nodes while building the DFA. That happens after parsing and without the reader, so the limits the injected reader carries never reach it, and a compact schema with a large maxOccurs was expanded unbounded on an implementation whose limit is opt-in: external Xerces applies it only under FEATURE_SECURE_PROCESSING, which the wrapper did not set (the stock JDK applies it unconditionally). Set FEATURE_SECURE_PROCESSING on the wrapped factory, failing closed as the other recipes do. The JAXP 1.5 ACCESS_EXTERNAL_* properties stay unset, so the caller-resolver opt-in path is unaffected; the schema tests covering it pass on Java 8 as well. The expansion is lazy on Xerces, so the new test validates an instance rather than only compiling the schema, and skips its unbounded control where the implementation applies the limit unconditionally. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CLnTBsvmYtxzNTWVGNyz33 --- src/changes/changes.xml | 1 + .../xml/secure/SecureSchemaFactory.java | 46 ++++++--- .../secure/SchemaContentModelLimitTest.java | 94 +++++++++++++++++++ 3 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 3f7a1c02..c1a755b6 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -48,6 +48,7 @@ The type attribute can be add, update, fix, or remove. Parse a Source opted in by a caller-supplied URIResolver using a secure parser. Secure the document parse behind the InputSource-taking XPath evaluation entry points. Fall back to the standard factory lookup in the DOM, SAX and schema newDefaultInstance methods on Android. + Bound the content model a schema expands into, so a compact schema with a large maxOccurs cannot exhaust memory or CPU during validation. Recognize XML implementations by the JAXP features and properties they support instead of by their implementation class name, extending the securing to any compliant implementation. Define a consistent contract for denied external fetches: unresolved external references resolve to empty content on every implementation, unless the org.apache.commons.xml.secure.throwOnUnresolved system property requests rejection. diff --git a/src/main/java/org/apache/commons/xml/secure/SecureSchemaFactory.java b/src/main/java/org/apache/commons/xml/secure/SecureSchemaFactory.java index 7aaa4c1c..05d471ac 100644 --- a/src/main/java/org/apache/commons/xml/secure/SecureSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/secure/SecureSchemaFactory.java @@ -40,8 +40,11 @@ * Beyond the three universal guarantees on {@link org.apache.commons.xml.secure}: *

*
    - *
  • {@code xs:import}, {@code xs:include} and {@code xs:redefine} schemaLocation URIs are not resolved during schema compilation, and
    • - *
  • {@code xsi:schemaLocation} / {@code xsi:noNamespaceSchemaLocation} hints in instance documents are not resolved during validation.
    • + *
  • {@code xs:import}, {@code xs:include} and {@code xs:redefine} schemaLocation URIs are not resolved during schema compilation,
    • + *
  • {@code xsi:schemaLocation} / {@code xsi:noNamespaceSchemaLocation} hints in instance documents are not resolved during validation, and
    • + *
  • the content model a schema expands into is bounded, on every implementation offering a limit for it. A loader expands a repeated particle while building + * the DFA, so a compact schema carrying a large {@code maxOccurs} would otherwise exhaust memory or CPU (see Xerces' + * security manager, which caps that expansion at 3,000 nodes).
    • *
*

* The same guarantees apply to {@link javax.xml.validation.Validator} and {@link javax.xml.validation.ValidatorHandler} instances produced from the @@ -58,8 +61,8 @@ public final class SecureSchemaFactory { /** * Capability-driven secure wrapper for any {@link SchemaFactory} on the classpath, the same recipe for every implementation. It is the entry point reached - * by {@link SecureSchemaFactory#newInstance(String)}; there is no per-implementation branching, no {@code FEATURE_SECURE_PROCESSING} and no limit configuration on the - * factory itself. + * by {@link SecureSchemaFactory#newInstance(String)}; there is no per-implementation branching and no limit configuration on the factory itself beyond + * {@code FEATURE_SECURE_PROCESSING}. * *

Three layers cooperate:

*
    @@ -73,9 +76,12 @@ public final class SecureSchemaFactory { * *

    * The secure reader supplied by {@link SecureSAXParserFactory#secure(Source, boolean)} already carries {@code FEATURE_SECURE_PROCESSING} and the processing limits, so a - * DOCTYPE, external entity or Billion Laughs payload in the schema or instance document is bounded there rather than on this factory. The JAXP 1.5 - * {@code ACCESS_EXTERNAL_*} properties are deliberately not set: the resolver floor already blocks the same fetches on every implementation, and the JDK 8 - * {@code SchemaFactory} has a bug whereby those properties keep blocking even when a caller's own resolver would grant the access. The floor is a non-removable + * DOCTYPE, external entity or Billion Laughs payload in the schema or instance document is bounded there rather than on this factory. One limit it cannot + * supply is content-model expansion: a large {@code maxOccurs} is expanded by the schema loader when it builds the DFA, after parsing and without the + * reader, so {@code FEATURE_SECURE_PROCESSING} is set on the factory as well, which is what installs that bound on external Xerces (the stock JDK applies + * it unconditionally). The JAXP 1.5 {@code ACCESS_EXTERNAL_*} properties are still not set explicitly: the resolver floor already blocks the same fetches on + * every implementation, and the JDK 8 {@code SchemaFactory} has a bug whereby those properties keep blocking even when a caller's own resolver would grant + * the access. The floor is a non-removable * lower bound: a caller-set {@link LSResourceResolver} is routed through it (opting a specific lookup in by returning a non-{@code null} result) rather than * replacing it, so secure cannot be dropped by swapping the resolver. *

    @@ -97,6 +103,8 @@ private static final class Wrapper extends SchemaFactory { */ private Wrapper(final SchemaFactory delegate) { this.delegate = Objects.requireNonNull(delegate, "delegate"); + // Content-model expansion happens in the schema loader, after parsing, so the injected reader's limits cannot reach it. + SecureSchemaFactory.setFeature(delegate, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Compile-time block for xs:import/include/redefine; the wrappers carry the rest (per-product resolver, source rewriting, limits via the reader). delegate.setResourceResolver(floor); } @@ -261,10 +269,10 @@ public static SchemaFactory newInstance(final String schemaLanguage, final Strin /** * Secures a {@link SchemaFactory}. * - *

    Unlike the other factory types there is no per-implementation branching and no feature or limit configuration on the factory itself: schema compilation - * and validation reach external resources only through the resolver hook, so wrapping the factory with a non-removable ignore-all resolver floor is enough on - * every implementation. The reader used to parse schema and instance documents is secure separately, through - * {@link SecureSAXParserFactory#secure(javax.xml.transform.Source, boolean)}.

    + *

    Unlike the other factory types there is no per-implementation branching: schema compilation and validation reach external resources only through the + * resolver hook, so wrapping the factory with a non-removable ignore-all resolver floor is enough on every implementation. The reader used to parse schema + * and instance documents is secure separately, through {@link SecureSAXParserFactory#secure(javax.xml.transform.Source, boolean)}; the factory carries + * {@code FEATURE_SECURE_PROCESSING} for the one limit that reader cannot supply, the loader's content-model expansion.

    * * @param factory the factory to secure; never {@code null}. * @return a secure factory. @@ -273,6 +281,22 @@ static SchemaFactory secure(final SchemaFactory factory) { return new Wrapper(factory); } + /** + * Sets a feature on the delegate, failing closed: an implementation that cannot accept it yields no factory rather than an unsecured one. + * + * @param factory the factory to configure; never {@code null}. + * @param feature the feature name. + * @param value the value to set. + * @throws SecureException if the implementation rejects the feature. + */ + private static void setFeature(final SchemaFactory factory, final String feature, final boolean value) { + try { + factory.setFeature(feature, value); + } catch (final Exception e) { + throw SecureException.featureFailed(feature, factory, e); + } + } + private SecureSchemaFactory() { // static only } diff --git a/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java b/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java new file mode 100644 index 00000000..93245bf1 --- /dev/null +++ b/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java @@ -0,0 +1,94 @@ +/* + * 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 + * + * https://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 org.apache.commons.xml.secure; + +import javax.xml.XMLConstants; +import javax.xml.validation.Schema; +import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Checks that an untrusted schema's content-model expansion is bounded, the one processing limit no reader can supply. + * + *

    {@link BillionLaughsTest} covers entity expansion, which the secure reader injected into every {@code Source} bounds before a schema document reaches the + * loader. {@code maxOccurs} is a different mechanism: the loader expands a repeated particle into content-model nodes while building the DFA, which happens + * after parsing and never touches the reader. The bound for it is the schema implementation's own limit ({@code maxOccurLimit}, 3,000 nodes on Xerces), which + * external Xerces installs only when {@code FEATURE_SECURE_PROCESSING} is set on the {@link SchemaFactory}.

    + * + *

    The expansion is lazy on Xerces: {@code newSchema} returns in milliseconds whatever {@code maxOccurs} says, and the nodes are built on first validation. + * The payload therefore has to be validated, not just compiled, and the assertion accepts a rejection at either step. The repeated particle holds two elements + * so it cannot be collapsed into Xerces' compact repeating-leaf form, and {@link #MAX_OCCURS} clears both limits by little enough that an unbounded run still + * finishes, in seconds, rather than exhausting the heap.

    + */ +@Tag("schema") +class SchemaContentModelLimitTest { + + /** Above both recognized implementations' limits (3,000 nodes on Xerces, 5,000 on the stock JDK); an unbounded run still finishes in seconds. */ + private static final int MAX_OCCURS = 10_000; + + private static String maxOccursPayload() { + return "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "\n"; + } + + /** Compiles the payload through {@code factory} and validates a matching instance, the step that forces the expansion. */ + private static void compileAndValidate(final SchemaFactory factory) throws Exception { + factory.setErrorHandler(AttackTestSupport.STRICT_REPORTER); + final Schema schema = factory.newSchema(AttackTestSupport.streamSource(maxOccursPayload())); + final Validator validator = schema.newValidator(); + validator.setErrorHandler(AttackTestSupport.STRICT_REPORTER); + validator.validate(AttackTestSupport.streamSource("xy")); + } + + @Test + void secureSchemaBoundsContentModelExpansion() { + AttackTestSupport.assertParseFails(() -> compileAndValidate(SecureSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)), + "Schema content-model expansion", org.xml.sax.SAXException.class); + } + + @Test + void unconfiguredSchemaWithSecureProcessingBoundsContentModelExpansion() { + // Control: the payload does trip the limit once secure processing is on, so a pass above is the limit firing rather than the payload being harmless. + AttackTestSupport.assertParseFails(() -> { + final SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + compileAndValidate(factory); + }, "Schema content-model expansion", org.xml.sax.SAXException.class); + } + + @Test + void unconfiguredSchemaValidatesWhereTheLimitIsOptional() { + // Control: the payload is a valid schema and instance, so a rejection above is the limit firing and not a malformed fixture. It is skipped on an + // implementation that bounds the expansion unconditionally (the stock JDK), where there is no unbounded run to compare against. + final SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + AttackTestSupport.assumeDoesNotThrow(() -> compileAndValidate(factory)); + } +} From 3148110590a8633682dbc42a299711f3c209d1f2 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Tue, 1 Sep 2026 07:32:17 -0400 Subject: [PATCH 2/2] Clarify test description in SchemaContentModelLimitTest Updated the test class description for clarity. --- .../apache/commons/xml/secure/SchemaContentModelLimitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java b/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java index 93245bf1..370c6d0f 100644 --- a/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java @@ -26,7 +26,7 @@ import org.junit.jupiter.api.Test; /** - * Checks that an untrusted schema's content-model expansion is bounded, the one processing limit no reader can supply. + * Tests that an untrusted schema's content-model expansion is bounded, the one processing limit no reader can supply. * *

    {@link BillionLaughsTest} covers entity expansion, which the secure reader injected into every {@code Source} bounds before a schema document reaches the * loader. {@code maxOccurs} is a different mechanism: the loader expands a repeated particle into content-model nodes while building the DFA, which happens