Bound the content model a schema expands into

src/changes/changes.xml

@@ -48,6 +48,7 @@ The <action> type attribute can be add, update, fix, or remove.
       <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Parse a Source opted in by a caller-supplied URIResolver using a secure parser.</action>
       <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Secure the document parse behind the InputSource-taking XPath evaluation entry points.</action>
       <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Fall back to the standard factory lookup in the DOM, SAX and schema newDefaultInstance methods on Android.</action>
+      <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Bound the content model a schema expands into, so a compact schema with a large maxOccurs cannot exhaust memory or CPU during validation.</action>
       <!-- UPDATE -->
       <action type="update" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory" issue="COMMONSXML-1,COMMONSXML-5,COMMONSXML-6,COMMONSXML-7,COMMONSXML-8">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.</action>
       <action type="update" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory" issue="COMMONSXML-4">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.</action>

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}:
  * </p>
  * <ul>
- * <li>{@code xs:import}, {@code xs:include} and {@code xs:redefine} schemaLocation URIs are not resolved during schema compilation, and</li>
- * <li>{@code xsi:schemaLocation} / {@code xsi:noNamespaceSchemaLocation} hints in instance documents are not resolved during validation.</li>
+ * <li>{@code xs:import}, {@code xs:include} and {@code xs:redefine} schemaLocation URIs are not resolved during schema compilation,</li>
+ * <li>{@code xsi:schemaLocation} / {@code xsi:noNamespaceSchemaLocation} hints in instance documents are not resolved during validation, and</li>
+ * <li>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'
+ * <a href="https://xerces.apache.org/xerces2-j/properties.html#security-manager">security manager</a>, which caps that expansion at 3,000 nodes).</li>
  * </ul>
  * <p>
  * 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}.
      *
      * <p>Three layers cooperate:</p>
      * <ol>
@@ -73,9 +76,12 @@ public final class SecureSchemaFactory {
      *
      * <p>
      * 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.
      * </p>
@@ -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}.
      *
-     * <p>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)}.</p>
+     * <p>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.</p>
      *
      * @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
     }

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;
+
+/**
+ * Tests that an untrusted schema's content-model expansion is bounded, the one processing limit no reader can supply.
+ *
+ * <p>{@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}.</p>
+ *
+ * <p>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.</p>
+ */
+@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 "<?xml version=\"1.0\"?>\n"
+                + "<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\">\n"
+                + "  <xs:element name=\"root\" type=\"bomb\"/>\n"
+                + "  <xs:complexType name=\"bomb\">\n"
+                + "    <xs:sequence>\n"
+                + "      <xs:sequence minOccurs=\"0\" maxOccurs=\"" + MAX_OCCURS + "\">\n"
+                + "        <xs:element name=\"a\" type=\"xs:string\"/>\n"
+                + "        <xs:element name=\"b\" type=\"xs:string\"/>\n"
+                + "      </xs:sequence>\n"
+                + "    </xs:sequence>\n"
+                + "  </xs:complexType>\n"
+                + "</xs:schema>\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("<root><a>x</a><b>y</b></root>"));
+    }
+
+    @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));
+    }
+}