COMMONSXML-7: SchemaFactory: capability-driven hardening

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<Validator> validatorHardener;
-    private final UnaryOperator<ValidatorHandler> handlerHardener;
 
-    HardeningSchema(final Schema delegate, final UnaryOperator<Validator> validatorHardener, final UnaryOperator<ValidatorHandler> 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;
     }
 }

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.
  *
  * <p>Three layers cooperate:</p>
  * <ol>
- *   <li>{@link HardeningSchemaFactory} rewrites the Source on every {@code newSchema(Source[])} entry point.</li>
- *   <li>{@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).</li>
+ *   <li>{@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)}.</li>
+ *   <li>{@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}.</li>
  *   <li>{@link HardeningValidator} rewrites the Source on every {@link Validator#validate(Source)} call.</li>
  * </ol>
+ *
+ * <p>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.</p>
  */
 final class HardeningSchemaFactory extends DelegatingSchemaFactory {
 
-    private final UnaryOperator<Validator> validatorHardener;
-    private final UnaryOperator<ValidatorHandler> handlerHardener;
-
     HardeningSchemaFactory(final SchemaFactory delegate) {
-        this(delegate, UnaryOperator.identity(), UnaryOperator.identity());
-    }
-
-    HardeningSchemaFactory(final SchemaFactory delegate, final UnaryOperator<Validator> validatorHardener,
-            final UnaryOperator<ValidatorHandler> 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 {

src/main/java/org/apache/commons/xml/HardeningValidator.java

@@ -32,14 +32,18 @@
 
 /**
  * {@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 {
 
     private final Validator delegate;
 
     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

src/main/java/org/apache/commons/xml/JaxpSetters.java

@@ -18,7 +18,6 @@
 package org.apache.commons.xml;
 
 import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.transform.TransformerFactory;
@@ -118,59 +117,35 @@ static void setOptionalProperty(final XMLInputFactory factory, final String prop
         trySetProperty(factory, property, value);
     }
 
-    static void setProperty(final XMLInputFactory factory, final String property, final Object value) {
-        apply(factory, KIND_PROPERTY, property, () -> factory.setProperty(property, value));
-    }
-
-    static void setProperty(final SAXParser parser, final String property, final Object value) {
-        apply(parser, KIND_PROPERTY, property, () -> parser.setProperty(property, value));
-    }
-
-    static void setProperty(final XMLReader reader, final String property, final Object value) {
-        apply(reader, KIND_PROPERTY, property, () -> reader.setProperty(property, value));
-    }
-
     /**
-     * Sets a property on an {@link XMLReader} and returns whether the implementation accepted it.
+     * Sets an attribute on a {@link DocumentBuilderFactory} and returns whether the implementation accepted it. Some implementations may reject certain
+     * attributes, in which case this method will return {@code false}.
      *
-     * @param reader   The target reader on which to set the property.
-     * @param property The name of the property to set.
-     * @param value    The value of the property to set.
-     * @return {@code true} if the property was applied, {@code false} if the implementation rejected it.
+     * @param factory   The target factory on which to set the attribute.
+     * @param attribute The name of the attribute to set.
+     * @param value     The value of the attribute to set.
+     * @return {@code true} if the attribute was applied, {@code false} if the implementation rejected it.
      */
-    static boolean trySetProperty(final XMLReader reader, final String property, final Object value) {
+    static boolean trySetAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
         try {
-            reader.setProperty(property, value);
+            factory.setAttribute(attribute, value);
             return true;
         } catch (final Exception e) {
             return false;
         }
     }
 
-    static void setProperty(final SchemaFactory factory, final String property, final Object value) {
-        apply(factory, KIND_PROPERTY, property, () -> factory.setProperty(property, value));
-    }
-
-    static void setProperty(final Validator validator, final String property, final Object value) {
-        apply(validator, KIND_PROPERTY, property, () -> validator.setProperty(property, value));
-    }
-
-    static void setProperty(final ValidatorHandler handler, final String property, final Object value) {
-        apply(handler, KIND_PROPERTY, property, () -> handler.setProperty(property, value));
-    }
-
     /**
-     * Sets an attribute on a {@link DocumentBuilderFactory} and returns whether the implementation accepted it. Some implementations may reject certain
-     * attributes, in which case this method will return {@code false}.
+     * Sets a property on an {@link XMLReader} and returns whether the implementation accepted it.
      *
-     * @param factory   The target factory on which to set the attribute.
-     * @param attribute The name of the attribute to set.
-     * @param value     The value of the attribute to set.
-     * @return {@code true} if the attribute was applied, {@code false} if the implementation rejected it.
+     * @param reader   The target reader on which to set the property.
+     * @param property The name of the property to set.
+     * @param value    The value of the property to set.
+     * @return {@code true} if the property was applied, {@code false} if the implementation rejected it.
      */
-    static boolean trySetAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
+    static boolean trySetProperty(final XMLReader reader, final String property, final Object value) {
         try {
-            factory.setAttribute(attribute, value);
+            reader.setProperty(property, value);
             return true;
         } catch (final Exception e) {
             return false;