COMMONSXML-1: DocumentBuilderFactory: capability-driven hardening

android-tests/build.gradle.kts

@@ -16,6 +16,7 @@
  */
 
 import com.android.build.api.dsl.ManagedVirtualDevice
+import org.gradle.api.tasks.compile.JavaCompile
 
 plugins {
     id("com.android.library") version "8.6.1"

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

@@ -172,10 +172,6 @@ public void setFeature(final String name, final boolean value) throws SAXNotReco
 
     private static final String NAMESPACE_PREFIXES_FEATURE = "http://xml.org/sax/features/namespace-prefixes";
 
-    static DocumentBuilderFactory configure(final DocumentBuilderFactory factory) {
-        return factory;
-    }
-
     static SAXParserFactory configure(final SAXParserFactory factory) {
         return new GuardedSAXParserFactory(factory);
     }

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

@@ -0,0 +1,103 @@
+/*
+ * 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;
+
+import static org.apache.commons.xml.JaxpSetters.setFeature;
+import static org.apache.commons.xml.JaxpSetters.setOptionalFeature;
+import static org.apache.commons.xml.JaxpSetters.trySetAttribute;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.xml.sax.EntityResolver;
+
+/**
+ * Capability-driven hardening for any {@link DocumentBuilderFactory} on the classpath.
+ *
+ * <p>Rather than branching on the implementation class, {@link #harden(DocumentBuilderFactory)} probes what the factory supports and adapts:</p>
+ * <ul>
+ *     <li><strong>Android</strong> (Harmony / KXmlParser): recognised by class name and left untouched. It exposes no {@link XMLConstants#FEATURE_SECURE_PROCESSING
+ *         FSP}, no JAXP 1.5 {@code ACCESS_EXTERNAL_*} and no attribute API at all, while KXmlParser silently drops user-defined entities, so there is nothing to
+ *         apply.</li>
+ *     <li><strong>FSP</strong>: required. It switches on the implementation's built-in security manager, which is what carries the processing limits.</li>
+ *     <li><strong>{@code XERCES_LOAD_EXTERNAL_DTD}</strong>: optional. Where supported, it skips the external DTD subset on non-validating parsers so a
+ *         DOCTYPE-only document parses without a fetch attempt. If not supported, the fetch will throw instead, due to the following settings.</li>
+ *     <li><strong>Limits</strong>: applied best-effort by {@link Limits#tryApply(DocumentBuilderFactory)}, which adapts to the JDK attribute limits or Xerces'
+ *         {@code SecurityManager} as appropriate.</li>
+ *     <li><strong>{@code ACCESS_EXTERNAL_DTD}</strong>: the dividing capability. Implementations that honour it (the JDK-internal Xerces) block external fetches
+ *         through the JAXP 1.5 properties and are returned as-is. Implementations that reject it (the external Xerces distribution) are wrapped so a deny-all
+ *         {@link EntityResolver} is installed on every {@link DocumentBuilder} produced.</li>
+ * </ul>
+ */
+final class DocumentBuilderHardener {
+
+    /**
+     * Wrapper that sets a deny-all {@link EntityResolver} on every {@link DocumentBuilder} produced.
+     *
+     * <p>Required for implementations that do not honour JAXP 1.5 {@code ACCESS_EXTERNAL_*} (the external Xerces distribution): the factory carries no resolver
+     * of its own, so it has to be set on each builder.</p>
+     */
+    private static final class HardeningDocumentBuilderFactory extends DelegatingDocumentBuilderFactory {
+
+        private final EntityResolver resolver;
+
+        HardeningDocumentBuilderFactory(final DocumentBuilderFactory delegate, final EntityResolver resolver) {
+            super(delegate);
+            this.resolver = resolver;
+        }
+
+        @Override
+        public DocumentBuilder newDocumentBuilder() throws ParserConfigurationException {
+            final DocumentBuilder builder = super.newDocumentBuilder();
+            builder.setEntityResolver(resolver);
+            return builder;
+        }
+    }
+
+    /** Class name of Android's Harmony-based {@link DocumentBuilderFactory}, which exposes no hardening surface. */
+    private static final String ANDROID_DOCUMENT_BUILDER_FACTORY = "org.apache.harmony.xml.parsers.DocumentBuilderFactoryImpl";
+
+    /** Xerces feature: load the external DTD subset for non-validating parsers. */
+    private static final String XERCES_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
+
+    static DocumentBuilderFactory harden(final DocumentBuilderFactory factory) {
+        // Android exposes no FSP, ACCESS_EXTERNAL_* or attribute API, and KXmlParser drops user-defined entities; nothing to apply.
+        if (ANDROID_DOCUMENT_BUILDER_FACTORY.equals(factory.getClass().getName())) {
+            return factory;
+        }
+        // Required: enables the implementation's security manager, which carries the limits.
+        setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        // Optional, implementation-based: JDK attribute limits or Xerces' SecurityManager.
+        Limits.tryApply(factory);
+        // Optional: skip the external DTD subset on non-validating parsers so DOCTYPE-only documents parse without a blocked fetch attempt.
+        setOptionalFeature(factory, XERCES_LOAD_EXTERNAL_DTD, false);
+        // ACCESS_EXTERNAL_* support is the dividing capability between JAXP 1.5 implementations and older ones.
+        if (trySetAttribute(factory, XMLConstants.ACCESS_EXTERNAL_DTD, "")
+                && trySetAttribute(factory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "")) {
+            // Honoured: the JAXP 1.5 properties block external fetches, so the bare factory is already hardened.
+            return factory;
+        }
+        // Rejected: external Xerces ignores ACCESS_EXTERNAL_*; install a deny-all resolver on every DocumentBuilder.
+        return new HardeningDocumentBuilderFactory(factory, Resolvers.DenyAll.ENTITY2);
+    }
+
+    private DocumentBuilderHardener() {
+    }
+}

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

@@ -55,6 +55,20 @@ static void setAttribute(final DocumentBuilderFactory factory, final String attr
         apply(factory, "attribute", attribute, () -> factory.setAttribute(attribute, value));
     }
 
+    /** @return {@code true} if the attribute was applied, {@code false} if the implementation rejected it. */
+    static boolean trySetAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
+        try {
+            factory.setAttribute(attribute, value);
+            return true;
+        } catch (final Exception e) {
+            return false;
+        }
+    }
+
+    static void setOptionalAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
+        trySetAttribute(factory, attribute, value);
+    }
+
     static void setAttribute(final TransformerFactory factory, final String attribute, final Object value) {
         apply(factory, "attribute", attribute, () -> factory.setAttribute(attribute, value));
     }
@@ -63,6 +77,14 @@ static void setFeature(final DocumentBuilderFactory factory, final String featur
         apply(factory, "feature", feature, () -> factory.setFeature(feature, value));
     }
 
+    static void setOptionalFeature(final DocumentBuilderFactory factory, final String feature, final boolean value) {
+        try {
+            factory.setFeature(feature, value);
+        } catch (final Exception e) {
+            // Ignored: the implementation does not recognise this feature.
+        }
+    }
+
     static void setFeature(final SAXParserFactory factory, final String feature, final boolean value) {
         apply(factory, "feature", feature, () -> factory.setFeature(feature, value));
     }

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

@@ -17,6 +17,7 @@
 package org.apache.commons.xml;
 
 import static org.apache.commons.xml.JaxpSetters.setAttribute;
+import static org.apache.commons.xml.JaxpSetters.setOptionalAttribute;
 import static org.apache.commons.xml.JaxpSetters.setProperty;
 
 import java.util.Collections;
@@ -216,6 +217,10 @@ final class Limits {
      * Woodstox property: maximum number of entity expansions in a single parse.
      */
     private static final String WSTX_MAX_ENTITY_COUNT = "com.ctc.wstx.maxEntityCount";
+    /**
+     * Class name of the external Apache Xerces {@link DocumentBuilderFactory}, whose limits live on a {@code SecurityManager} rather than JDK attributes.
+     */
+    private static final String EXTERNAL_XERCES_DOCUMENT_BUILDER_FACTORY = "org.apache.xerces.jaxp.DocumentBuilderFactoryImpl";
 
     static {
         final Map<String, IntSupplier> map = new LinkedHashMap<>();
@@ -231,12 +236,23 @@ final class Limits {
     }
 
     /**
-     * Sets every JDK-supported limit on a stock JDK {@link DocumentBuilderFactory}.
+     * Best-effort application of the processing limits to a {@link DocumentBuilderFactory}, dispatched on the implementation.
+     *
+     * <p>External Xerces carries its limits on an {@code org.apache.xerces.util.SecurityManager} instance. Every other implementation (the stock JDK and any
+     * future attribute-based parser) takes the JDK limit attributes. Neither path throws if the implementation declines a limit.</p>
      *
      * @param factory The target factory to modify.
      */
-    static void applyToJdkDom(final DocumentBuilderFactory factory) {
-        JDK_LIMITS.forEach((name, supplier) -> setAttribute(factory, name, Integer.toString(supplier.getAsInt())));
+    static void tryApply(final DocumentBuilderFactory factory) {
+        if (EXTERNAL_XERCES_DOCUMENT_BUILDER_FACTORY.equals(factory.getClass().getName())) {
+            // Install a fresh SecurityManager pinned to JDK 25 limits, replacing Xerces' built-in caps which are looser than even JDK 8.
+            final Object securityManager = newSecurityManager();
+            applyToXerces(securityManager);
+            setAttribute(factory, XercesProvider.XERCES_SECURITY_MANAGER_PROPERTY, securityManager);
+            return;
+        }
+        // Pin the JDK attribute limits to JDK 25 secure values; skip silently any attribute the implementation does not recognise.
+        JDK_LIMITS.forEach((name, supplier) -> setOptionalAttribute(factory, name, Integer.toString(supplier.getAsInt())));
     }
 
     /**
@@ -304,6 +320,14 @@ static void applyToXerces(final Object securityManager) {
         }
     }
 
+    private static Object newSecurityManager() {
+        try {
+            return Class.forName("org.apache.xerces.util.SecurityManager").getDeclaredConstructor().newInstance();
+        } catch (final ReflectiveOperationException e) {
+            throw new HardeningException("Failed to instantiate org.apache.xerces.util.SecurityManager; expected Xerces to be on the classpath", e);
+        }
+    }
+
     private static int getElementAttributeLimit() {
         return read(SP_ELEMENT_ATTRIBUTE_LIMIT, DEFAULT_ELEMENT_ATTRIBUTE_LIMIT);
     }

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

@@ -22,7 +22,6 @@
 import static org.apache.commons.xml.JaxpSetters.setProperty;
 
 import javax.xml.XMLConstants;
-import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.SAXParserFactory;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.transform.TransformerFactory;
@@ -72,19 +71,6 @@ final class StockJdkProvider {
      */
     private static final String ZEPHYR_IGNORE_EXTERNAL_DTD = "http://java.sun.com/xml/stream/properties/ignore-external-dtd";
 
-    static DocumentBuilderFactory configure(final DocumentBuilderFactory factory) {
-        // Required: enables the JDK XMLSecurityManager limits.
-        setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        // Let DOCTYPE-only documents parse silently without SSRF: skip the external DTD subset on non-validating parsers.
-        setFeature(factory, XERCES_LOAD_EXTERNAL_DTD, false);
-        // Defense-in-depth: pin to JDK 25 limits so older JDKs do not fall back to looser secure values.
-        Limits.applyToJdkDom(factory);
-        // Defense-in-depth: already FSP-secure defaults, set explicitly so they are not relaxed via system property.
-        setAttribute(factory, XMLConstants.ACCESS_EXTERNAL_DTD, "");
-        setAttribute(factory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
-        return factory;
-    }
-
     static SAXParserFactory configure(final SAXParserFactory factory) {
         // Required: enables the JDK XMLSecurityManager limits.
         setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);

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

@@ -20,17 +20,13 @@
 import static org.apache.commons.xml.JaxpSetters.setFeature;
 
 import javax.xml.XMLConstants;
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
 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.EntityResolver;
 import org.xml.sax.SAXNotRecognizedException;
 import org.xml.sax.SAXNotSupportedException;
 import org.xml.sax.XMLReader;
@@ -39,7 +35,7 @@
  * Hardening recipes for the external Apache Xerces distribution (the {@code xerces:xercesImpl} artifact).
  *
  * <p>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 DOM, SAX and Schema factories.</p>
+ * {@code XPathFactory}, so this class only handles SAX and Schema factories. DOM hardening lives in {@link DocumentBuilderHardener}.</p>
  *
  * <p>Hardening recipe applied to every factory below uses the same building blocks:</p>
  * <ul>
@@ -53,8 +49,7 @@
  *         {@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:</p>
  *         <ol>
- *             <li>{@link DocumentBuilderFactory} / {@link SAXParserFactory} carry no resolver, so it has to be set on each
- *             {@link DocumentBuilder} / {@link SAXParser} produced;</li>
+ *             <li>{@link SAXParserFactory} carries no resolver, so it has to be set on each {@link SAXParser} produced.</li>
  *             <li>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.</li>
  *         </ol>
@@ -63,29 +58,6 @@
  */
 final class XercesProvider {
 
-    /**
-     * Hardened Xerces {@link DocumentBuilderFactory} wrapper.
-     *
-     * <p>Sets the deny-all {@link EntityResolver} on every {@link DocumentBuilder} produced; required because {@link DocumentBuilderFactory} carries no
-     * resolver of its own and Xerces does not honour JAXP 1.5 {@code ACCESS_EXTERNAL_*}.</p>
-     */
-    private static final class HardeningDocumentBuilderFactory extends DelegatingDocumentBuilderFactory {
-
-        private final EntityResolver resolver;
-
-        HardeningDocumentBuilderFactory(final DocumentBuilderFactory delegate, final EntityResolver resolver) {
-            super(delegate);
-            this.resolver = resolver;
-        }
-
-        @Override
-        public DocumentBuilder newDocumentBuilder() throws ParserConfigurationException {
-            final DocumentBuilder builder = super.newDocumentBuilder();
-            builder.setEntityResolver(resolver);
-            return builder;
-        }
-    }
-
     private static Validator hardenValidator(final Validator validator) {
         try {
             Limits.applyToXerces(validator.getProperty(XERCES_SECURITY_MANAGER_PROPERTY));
@@ -114,27 +86,6 @@ private static ValidatorHandler hardenValidatorHandler(final ValidatorHandler ha
     /** Xerces feature: load the external DTD subset for non-validating parsers. */
     private static final String XERCES_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
 
-    private static Object newSecurityManager() {
-        try {
-            return Class.forName("org.apache.xerces.util.SecurityManager").getDeclaredConstructor().newInstance();
-        } catch (final ReflectiveOperationException e) {
-            throw new HardeningException("Failed to instantiate org.apache.xerces.util.SecurityManager; expected Xerces to be on the classpath", e);
-        }
-    }
-
-    static DocumentBuilderFactory configure(final DocumentBuilderFactory factory) {
-        // Required: enables Xerces' built-in SecurityManager (which is what carries the limits).
-        setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        // Let DOCTYPE-only documents parse silently without SSRF: skip the external DTD subset on non-validating parsers.
-        setFeature(factory, XERCES_LOAD_EXTERNAL_DTD, false);
-        // Defense-in-depth: install a fresh SecurityManager pinned to JDK 25 limits, replacing Xerces' built-in caps which are looser than even JDK 8.
-        final Object securityManager = newSecurityManager();
-        Limits.applyToXerces(securityManager);
-        factory.setAttribute(XERCES_SECURITY_MANAGER_PROPERTY, securityManager);
-        // Required: Xerces does not honour JAXP 1.5 ACCESS_EXTERNAL_*; the wrapper installs a deny-all resolver on every DocumentBuilder.
-        return new HardeningDocumentBuilderFactory(factory, Resolvers.DenyAll.ENTITY2);
-    }
-
     static SAXParserFactory configure(final SAXParserFactory factory) {
         // Required: enables Xerces' built-in SecurityManager (which is what carries the limits).
         setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true);