From 2f44caa2c3182fd5ae29fd87ec7814ccf8846e30 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 28 Jun 2026 23:16:48 +0200 Subject: [PATCH] Base SchemaFactory hardening on feature support Replace the per-implementation class-name dispatch for Schema with a single wrapper-based recipe, the same for every implementation. The hardening lives entirely in the existing HardeningSchemaFactory / HardeningSchema / HardeningValidator wrappers, so there is no per-implementation branching, no FEATURE_SECURE_PROCESSING and no limit configuration on the factory itself: - HardeningSchemaFactory installs a deny-all LSResourceResolver on the factory (blocking xs:import/include/redefine at compile time) and rewrites every newSchema(Source[]) through an XmlFactories-hardened reader. - HardeningSchema wraps every Validator/ValidatorHandler the inner Schema produces and re-installs the deny-all resolver on each (blocking xsi:schemaLocation at validation time), since neither the JDK nor Xerces reliably propagates it through Schema. - HardeningValidator rewrites the Source on every validate(Source) call. The hardened reader from XmlFactories.harden(Source) already carries 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 ACCESS_EXTERNAL_* properties are deliberately not set: the deny-all resolver already blocks the same fetches on every implementation, and the JDK 8 SchemaFactory has a bug whereby those properties keep blocking even when a caller's own resolver would grant the access, so leaving them unset lets a caller re-enable specific lookups by swapping the resolver. The JDK block now surfaces as the resolver's SecurityException rather than a SAXException, which the attack-test assertions already accept. StockJdkProvider.configure(SchemaFactory), XercesProvider.configure(SchemaFactory) with its per-product hardeners, XmlFactories.dispatch(SchemaFactory) and Limits.applyToJdkSchema are removed; XmlFactories.newSchemaFactory() wraps in HardeningSchemaFactory directly. An implementation is no longer rejected for being unrecognized. Assisted-By: Claude Opus 4.8 (1M context) --- .../apache/commons/xml/HardeningSchema.java | 21 +++---- .../commons/xml/HardeningSchemaFactory.java | 38 ++++++------- .../commons/xml/HardeningValidator.java | 6 +- .../java/org/apache/commons/xml/Limits.java | 10 ---- .../apache/commons/xml/StockJdkProvider.java | 14 ----- .../apache/commons/xml/XercesProvider.java | 56 ++----------------- .../org/apache/commons/xml/XmlFactories.java | 15 +---- 7 files changed, 38 insertions(+), 122 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/HardeningSchema.java b/src/main/java/org/apache/commons/xml/HardeningSchema.java index 3dcc109..5f0ad15 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchema.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchema.java @@ -17,36 +17,33 @@ package org.apache.commons.xml; -import java.util.function.UnaryOperator; - import javax.xml.validation.Schema; import javax.xml.validation.Validator; import javax.xml.validation.ValidatorHandler; /** - * {@link Schema} wrapper that applies provider-specific decoration to every {@link Validator} and {@link ValidatorHandler} the inner Schema produces, then - * wraps each {@link Validator} in {@link HardeningValidator} so {@link Validator#validate(javax.xml.transform.Source)} runs through - * {@link XmlFactories#harden(javax.xml.transform.Source)}. + * {@link Schema} wrapper that hardens every {@link Validator} and {@link ValidatorHandler} the inner Schema produces: each {@link Validator} is wrapped in + * {@link HardeningValidator} (which rewrites the Source through {@link XmlFactories#harden(javax.xml.transform.Source)} and installs the deny-all resolver), and + * each {@link ValidatorHandler} gets the same deny-all {@link Resolvers.DenyAll#LS_RESOURCE} so {@code xsi:schemaLocation} is not resolved during SAX-driven + * validation. */ final class HardeningSchema extends Schema { private final Schema delegate; - private final UnaryOperator validatorHardener; - private final UnaryOperator handlerHardener; - HardeningSchema(final Schema delegate, final UnaryOperator validatorHardener, final UnaryOperator handlerHardener) { + HardeningSchema(final Schema delegate) { this.delegate = delegate; - this.validatorHardener = validatorHardener; - this.handlerHardener = handlerHardener; } @Override public Validator newValidator() { - return new HardeningValidator(validatorHardener.apply(delegate.newValidator())); + return new HardeningValidator(delegate.newValidator()); } @Override public ValidatorHandler newValidatorHandler() { - return handlerHardener.apply(delegate.newValidatorHandler()); + final ValidatorHandler handler = delegate.newValidatorHandler(); + handler.setResourceResolver(Resolvers.DenyAll.LS_RESOURCE); + return handler; } } diff --git a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java index ece9494..e28f65e 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java @@ -17,53 +17,51 @@ package org.apache.commons.xml; -import java.util.function.UnaryOperator; - import javax.xml.transform.Source; import javax.xml.transform.TransformerConfigurationException; import javax.xml.validation.Schema; import javax.xml.validation.SchemaFactory; import javax.xml.validation.Validator; -import javax.xml.validation.ValidatorHandler; import org.xml.sax.SAXException; /** - * {@link SchemaFactory} wrapper that rewrites every Source-taking entry point through {@link XmlFactories#harden(Source)} and applies provider-specific - * decoration to every {@link Validator} and {@link ValidatorHandler} that the produced {@link Schema} hands out. + * Capability-driven hardening wrapper for any {@link SchemaFactory} on the classpath, the same recipe for every implementation. It is the entry point reached + * by {@link XmlFactories#newSchemaFactory()}; there is no per-implementation branching, no {@code FEATURE_SECURE_PROCESSING} and no limit configuration on the + * factory itself. * *

Three layers cooperate:

*
    - *
  1. {@link HardeningSchemaFactory} rewrites the Source on every {@code newSchema(Source[])} entry point.
    2. - *
  2. {@link HardeningSchema} applies the provider-specific hardener to every Validator/ValidatorHandler the inner Schema produces (e.g. Xerces re-installs - * its {@code LSResourceResolver} and {@code SecurityManager} since it does not propagate them through Schema).
    3. + *
  3. {@link HardeningSchemaFactory} installs a deny-all {@link Resolvers.DenyAll#LS_RESOURCE} on the factory (blocking + * {@code xs:import}/{@code xs:include}/{@code xs:redefine} at compile time) and rewrites the Source on every {@code newSchema(Source[])} entry point + * through {@link XmlFactories#harden(Source)}.
    4. + *
  4. {@link HardeningSchema} wraps every Validator/ValidatorHandler the inner Schema produces and re-installs the deny-all resolver on each (blocking + * {@code xsi:schemaLocation} at validation time), since neither the JDK nor Xerces reliably propagates it through {@code Schema}.
    5. *
  5. {@link HardeningValidator} rewrites the Source on every {@link Validator#validate(Source)} call.
    6. *
+ * + *

The hardened reader supplied by {@link XmlFactories#harden(Source)} 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 deny-all resolver 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, so leaving them unset lets a + * caller re-enable specific lookups by swapping the resolver.

*/ final class HardeningSchemaFactory extends DelegatingSchemaFactory { - private final UnaryOperator validatorHardener; - private final UnaryOperator handlerHardener; - HardeningSchemaFactory(final SchemaFactory delegate) { - this(delegate, UnaryOperator.identity(), UnaryOperator.identity()); - } - - HardeningSchemaFactory(final SchemaFactory delegate, final UnaryOperator validatorHardener, - final UnaryOperator handlerHardener) { super(delegate); - this.validatorHardener = validatorHardener; - this.handlerHardener = handlerHardener; + // Compile-time block for xs:import/include/redefine; the wrappers carry the rest (per-product resolver, source rewriting, limits via the reader). + delegate.setResourceResolver(Resolvers.DenyAll.LS_RESOURCE); } @Override public Schema newSchema() throws SAXException { - return new HardeningSchema(super.newSchema(), validatorHardener, handlerHardener); + return new HardeningSchema(super.newSchema()); } @Override public Schema newSchema(final Source[] schemas) throws SAXException { - return new HardeningSchema(super.newSchema(harden(schemas)), validatorHardener, handlerHardener); + return new HardeningSchema(super.newSchema(harden(schemas))); } private static Source[] harden(final Source[] schemas) throws SAXException { diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java b/src/main/java/org/apache/commons/xml/HardeningValidator.java index 3df9596..86dc063 100644 --- a/src/main/java/org/apache/commons/xml/HardeningValidator.java +++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java @@ -32,7 +32,8 @@ /** * {@link Validator} wrapper that rewrites the Source on every {@link Validator#validate(Source)} and {@link Validator#validate(Source, Result)} call through - * {@link XmlFactories#harden(Source)} before delegating. + * {@link XmlFactories#harden(Source)} before delegating, and installs a deny-all {@link LSResourceResolver} so {@code xsi:schemaLocation} is not resolved at + * validation time. */ final class HardeningValidator extends Validator { @@ -40,6 +41,9 @@ final class HardeningValidator extends Validator { HardeningValidator(final Validator delegate) { this.delegate = delegate; + // Block xsi:schemaLocation resolution; neither the JDK nor Xerces reliably propagates the factory's resolver to its Validators. A caller may re-enable + // specific lookups by setting their own resolver afterwards. + delegate.setResourceResolver(Resolvers.DenyAll.LS_RESOURCE); } @Override diff --git a/src/main/java/org/apache/commons/xml/Limits.java b/src/main/java/org/apache/commons/xml/Limits.java index d60c655..a85190f 100644 --- a/src/main/java/org/apache/commons/xml/Limits.java +++ b/src/main/java/org/apache/commons/xml/Limits.java @@ -28,7 +28,6 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.stream.XMLInputFactory; import javax.xml.transform.TransformerFactory; -import javax.xml.validation.SchemaFactory; import org.xml.sax.XMLReader; @@ -255,15 +254,6 @@ static void tryApply(final DocumentBuilderFactory factory) { JDK_LIMITS.forEach((name, supplier) -> setOptionalAttribute(factory, name, Integer.toString(supplier.getAsInt()))); } - /** - * Sets every JDK-supported limit on a stock JDK {@link SchemaFactory}. - * - * @param factory The target factory to modify. - */ - static void applyToJdkSchema(final SchemaFactory factory) { - JDK_LIMITS.forEach((name, supplier) -> setProperty(factory, name, Integer.toString(supplier.getAsInt()))); - } - /** * Sets every JDK-supported limit on the stock JDK's {@link XMLInputFactory}. * diff --git a/src/main/java/org/apache/commons/xml/StockJdkProvider.java b/src/main/java/org/apache/commons/xml/StockJdkProvider.java index 99a7d8d..d0bd272 100644 --- a/src/main/java/org/apache/commons/xml/StockJdkProvider.java +++ b/src/main/java/org/apache/commons/xml/StockJdkProvider.java @@ -26,7 +26,6 @@ import javax.xml.stream.XMLInputFactory; import javax.xml.transform.TransformerFactory; import javax.xml.transform.sax.SAXTransformerFactory; -import javax.xml.validation.SchemaFactory; import javax.xml.xpath.XPathFactory; import org.xml.sax.XMLReader; @@ -124,19 +123,6 @@ static XPathFactory configure(final XPathFactory factory) { return factory; } - static SchemaFactory configure(final SchemaFactory factory) { - // Required: enables the JDK XMLSecurityManager limits. - setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); - // Defense-in-depth: pin to JDK 25 limits so older JDKs do not fall back to looser secure values. - Limits.applyToJdkSchema(factory); - // Required: XMLSchemaLoader propagates this onto its inner SAX reader, otherwise it is overrideable by system properties - setProperty(factory, XMLConstants.ACCESS_EXTERNAL_DTD, ""); - // Required: gates xs:import/include/redefine fetches and xsi:schemaLocation. - setProperty(factory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); - // Required: routes every newSchema(Source[]) parse through an XmlFactories-hardened reader. - return new HardeningSchemaFactory(factory); - } - private StockJdkProvider() { } } diff --git a/src/main/java/org/apache/commons/xml/XercesProvider.java b/src/main/java/org/apache/commons/xml/XercesProvider.java index 4634c9f..950ce06 100644 --- a/src/main/java/org/apache/commons/xml/XercesProvider.java +++ b/src/main/java/org/apache/commons/xml/XercesProvider.java @@ -22,10 +22,6 @@ import javax.xml.XMLConstants; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; -import javax.xml.validation.Schema; -import javax.xml.validation.SchemaFactory; -import javax.xml.validation.Validator; -import javax.xml.validation.ValidatorHandler; import org.xml.sax.SAXNotRecognizedException; import org.xml.sax.SAXNotSupportedException; @@ -35,7 +31,8 @@ * Hardening recipes for the external Apache Xerces distribution (the {@code xerces:xercesImpl} artifact). * *

Factory classes live in the {@code org.apache.xerces.*} package. External Xerces does not ship a {@code TransformerFactory}, {@code XMLInputFactory} or - * {@code XPathFactory}, so this class only handles SAX and Schema factories. DOM hardening lives in {@link DocumentBuilderHardener}.

+ * {@code XPathFactory}, so this class only handles SAX factories; Schema hardening is capability-driven across all implementations and lives in + * {@link HardeningSchemaFactory}, DOM hardening in {@link DocumentBuilderHardener}.

* *

Hardening recipe applied to every factory below uses the same building blocks:

*
    @@ -44,40 +41,13 @@ *
  • {@link Limits#applyToXerces}: defense-in-depth. Xerces' {@code SecurityManager} ships its own caps, but they are looser than even * JDK 8's secure values; this call pins them to the JDK 25 secure values (entity-expansion limit and {@code maxOccurs} node limit, the only two its * API exposes setters for).
    • - *
  • - *

    {@code HardeningXxx} wrappers + {@link Resolvers.DenyAll}: required. Xerces does not implement the JAXP 1.5 - * {@code ACCESS_EXTERNAL_*} properties, so an explicit resolver installed on every parser/validator is the best way to block external - * entity, DTD and schema fetching, without disabling those features altogether. The wrappers exist for two reasons:

    - *
      - *
    1. {@link SAXParserFactory} carries no resolver, so it has to be set on each {@link SAXParser} produced.
      2. - *
    2. Xerces' {@link Schema} does not propagate the {@link SchemaFactory}'s resolver or security manager to its - * {@link Validator} / {@link ValidatorHandler} products, so the wrapper re-installs both on every product.
      3. - *
    - *
    • + *
  • {@link Resolvers.DenyAll#ENTITY2}: required. Xerces does not implement the JAXP 1.5 {@code ACCESS_EXTERNAL_*} properties, so an + * explicit deny-all resolver installed on every reader is the best way to block external entity and DTD fetching, without disabling those features + * altogether. {@link SAXParserFactory} carries no resolver, so it has to be set on each {@link SAXParser} produced.
    • *
*/ final class XercesProvider { - private static Validator hardenValidator(final Validator validator) { - try { - Limits.applyToXerces(validator.getProperty(XERCES_SECURITY_MANAGER_PROPERTY)); - } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { - throw new HardeningException("Failed to read Xerces security manager from Validator", e); - } - validator.setResourceResolver(Resolvers.DenyAll.LS_RESOURCE); - return validator; - } - - private static ValidatorHandler hardenValidatorHandler(final ValidatorHandler handler) { - try { - Limits.applyToXerces(handler.getProperty(XERCES_SECURITY_MANAGER_PROPERTY)); - } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { - throw new HardeningException("Failed to read Xerces security manager from ValidatorHandler", e); - } - handler.setResourceResolver(Resolvers.DenyAll.LS_RESOURCE); - return handler; - } - /** * Xerces-specific property whose value is an {@code org.apache.xerces.util.SecurityManager} instance carrying processing-limit thresholds */ @@ -111,22 +81,6 @@ static XMLReader configure(final XMLReader reader) { return reader; } - static SchemaFactory configure(final SchemaFactory factory) { - // Required: enables Xerces' built-in SecurityManager. - setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); - try { - // Required: pins limits to JDK 25 secure values, otherwise Xerces' own caps are looser than JDK 8. - Limits.applyToXerces(factory.getProperty(XERCES_SECURITY_MANAGER_PROPERTY)); - } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { - throw new HardeningException("Failed to read Xerces security manager from SchemaFactory", e); - } - // Required: Xerces ignores ACCESS_EXTERNAL_*; the deny-all resolver blocks xs:import/include/redefine fetches. - factory.setResourceResolver(Resolvers.DenyAll.LS_RESOURCE); - // Required: routes every newSchema(Source[]) parse through an XmlFactories-hardened reader, and re-installs limits + resolver on each Validator and - // ValidatorHandler since Xerces' Schema does not propagate factory state through. - return new HardeningSchemaFactory(factory, XercesProvider::hardenValidator, XercesProvider::hardenValidatorHandler); - } - private XercesProvider() { } } diff --git a/src/main/java/org/apache/commons/xml/XmlFactories.java b/src/main/java/org/apache/commons/xml/XmlFactories.java index e7a7953..40cd9d0 100644 --- a/src/main/java/org/apache/commons/xml/XmlFactories.java +++ b/src/main/java/org/apache/commons/xml/XmlFactories.java @@ -126,17 +126,6 @@ private static XPathFactory dispatch(final XPathFactory factory) { } } - private static SchemaFactory dispatch(final SchemaFactory factory) { - switch (factory.getClass().getName()) { - case "com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchemaFactory": - return StockJdkProvider.configure(factory); - case "org.apache.xerces.jaxp.validation.XMLSchemaFactory": - return XercesProvider.configure(factory); - default: - throw noProvider(factory); - } - } - /** * Rewrites a {@link Source} so that any SAX parsing it triggers runs through an {@link XmlFactories}-hardened {@link XMLReader}. * @@ -224,11 +213,9 @@ public static SAXParserFactory newSAXParserFactory() { * resulting {@link javax.xml.validation.Schema}.

* * @return a hardened factory. - * @throws IllegalStateException if the underlying Schema implementation is not recognized by any bundled hardening recipe, or if the matching recipe - * cannot apply its settings to it. */ public static SchemaFactory newSchemaFactory() { - return dispatch(SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)); + return new HardeningSchemaFactory(SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)); } /**