COMMONSXML-6: XMLInputFactory: capability-driven hardening

src/conf/spotbugs-exclude-filter.xml

@@ -19,9 +19,9 @@
     xmlns="https://github.com/spotbugs/filter/3.0.0"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">
-  <!-- Looks like a FP. -->
+  <!-- Looks like an NPE: the DTD_SUBSET_ONLY lambda forwards a known-null entityName to the ignore-all resolver. -->
   <Match>
-    <Class name="org.apache.commons.xml.WoodstoxProvider" />
+    <Class name="org.apache.commons.xml.StaxHardener" />
     <Method name="lambda$static$0" />
     <Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE" />
   </Match>

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

@@ -37,15 +37,14 @@
  */
 final class JaxpSetters {
 
-    private static final String KIND_PROPERTY = "property";
-    private static final String KIND_FEATURE = "feature";
-    private static final String KIND_ATTRIBUTE = "attribute";
-
     /** Action that may throw any exception; used to share a single try/catch around every JAXP setter. */
     @FunctionalInterface
     private interface ThrowingAction {
         void run() throws Exception;
     }
+    private static final String KIND_ATTRIBUTE = "attribute";
+    private static final String KIND_FEATURE = "feature";
+    private static final String KIND_PROPERTY = "property";
 
     private static void apply(final Object factory, final String kind, final String name, final ThrowingAction action) {
         try {
@@ -59,28 +58,6 @@ static void setAttribute(final DocumentBuilderFactory factory, final String attr
         apply(factory, KIND_ATTRIBUTE, attribute, () -> factory.setAttribute(attribute, 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}.
-     *
-     * @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 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, KIND_ATTRIBUTE, attribute, () -> factory.setAttribute(attribute, value));
     }
@@ -89,14 +66,6 @@ static void setFeature(final DocumentBuilderFactory factory, final String featur
         apply(factory, KIND_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 recognize this feature.
-        }
-    }
-
     static void setFeature(final SAXParserFactory factory, final String feature, final boolean value) {
         apply(factory, KIND_FEATURE, feature, () -> factory.setFeature(feature, value));
     }
@@ -125,6 +94,18 @@ static void setFeature(final XMLReader reader, final String feature, final boole
         apply(reader, KIND_FEATURE, feature, () -> reader.setFeature(feature, value));
     }
 
+    static void setOptionalAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
+        trySetAttribute(factory, attribute, 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 recognize this feature.
+        }
+    }
+
     static void setOptionalFeature(final XMLReader reader, final String feature, final boolean value) {
         try {
             reader.setFeature(feature, value);
@@ -133,6 +114,10 @@ static void setOptionalFeature(final XMLReader reader, final String feature, fin
         }
     }
 
+    static void setOptionalProperty(final XMLInputFactory factory, final String property, final Object value) {
+        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));
     }
@@ -174,6 +159,41 @@ static void setProperty(final ValidatorHandler handler, final String property, f
         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}.
+     *
+     * @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 trySetAttribute(final DocumentBuilderFactory factory, final String attribute, final Object value) {
+        try {
+            factory.setAttribute(attribute, value);
+            return true;
+        } catch (final Exception e) {
+            return false;
+        }
+    }
+
+    /**
+     * Sets a property on an {@link XMLInputFactory} and returns whether the implementation accepted it.
+     *
+     * @param factory   The target factory 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 trySetProperty(final XMLInputFactory factory, final String property, final Object value) {
+        try {
+            factory.setProperty(property, value);
+            return true;
+        } catch (final Exception e) {
+            return false;
+        }
+    }
+
     private JaxpSetters() {
     }
 }

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

@@ -242,26 +242,6 @@ final class Limits {
         JDK_LIMITS = Collections.unmodifiableMap(map);
     }
 
-    /**
-     * 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 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 recognize.
-        JDK_LIMITS.forEach((name, supplier) -> setOptionalAttribute(factory, name, Integer.toString(supplier.getAsInt())));
-    }
-
     /**
      * Sets every JDK-supported limit on a stock JDK {@link SchemaFactory}.
      *
@@ -271,15 +251,6 @@ 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}.
-     *
-     * @param factory The target factory to modify.
-     */
-    static void applyToJdkStax(final XMLInputFactory factory) {
-        JDK_LIMITS.forEach((name, supplier) -> setProperty(factory, name, Integer.toString(supplier.getAsInt())));
-    }
-
     /**
      * Sets every JDK-supported limit on a stock JDK {@link TransformerFactory}.
      *
@@ -313,17 +284,6 @@ static void tryApply(final XMLReader reader) {
         JDK_LIMITS.forEach((name, supplier) -> trySetProperty(reader, name, Integer.toString(supplier.getAsInt())));
     }
 
-    /**
-     * Sets every JDK-supported limit on a Woodstox {@link XMLInputFactory}.
-     *
-     * @param factory The target factory to modify.
-     */
-    static void applyToWoodstox(final XMLInputFactory factory) {
-        setProperty(factory, WSTX_MAX_ENTITY_COUNT, getEntityExpansionLimit());
-        setProperty(factory, WSTX_MAX_ATTRIBUTES_PER_ELEMENT, getElementAttributeLimit());
-        setProperty(factory, WSTX_MAX_ELEMENT_DEPTH, getMaxElementDepth());
-    }
-
     /**
      * Sets every JDK-supported limit on a Xerces {@code org.apache.xerces.util.SecurityManager}.
      *
@@ -398,6 +358,41 @@ private static int read(final String systemPropertyName, final int defaultValue)
         }
     }
 
+    /**
+     * 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 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 recognize.
+        JDK_LIMITS.forEach((name, supplier) -> setOptionalAttribute(factory, name, Integer.toString(supplier.getAsInt())));
+    }
+
+    /**
+     * Best-effort application of the processing limits to an {@link XMLInputFactory}, regardless of implementation.
+     *
+     * <p>The JDK's Zephyr honours the JDK URL limit properties; Woodstox honours its own {@code com.ctc.wstx.*} properties. Each implementation rejects the
+     * other's, so both sets are applied best-effort and the rejected ones are skipped silently.</p>
+     *
+     * @param factory The target factory to modify.
+     */
+    static void tryApply(final XMLInputFactory factory) {
+        JDK_LIMITS.forEach((name, supplier) -> trySetProperty(factory, name, Integer.toString(supplier.getAsInt())));
+        trySetProperty(factory, WSTX_MAX_ENTITY_COUNT, getEntityExpansionLimit());
+        trySetProperty(factory, WSTX_MAX_ATTRIBUTES_PER_ELEMENT, getElementAttributeLimit());
+        trySetProperty(factory, WSTX_MAX_ELEMENT_DEPTH, getMaxElementDepth());
+    }
+
     private Limits() {
     }
 }

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

@@ -0,0 +1,92 @@
+/*
+ * 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.setOptionalProperty;
+import static org.apache.commons.xml.JaxpSetters.trySetProperty;
+
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLResolver;
+import javax.xml.stream.XMLStreamException;
+
+/**
+ * Capability-driven hardening for any {@link XMLInputFactory} (StAX) on the classpath.
+ *
+ * <p>Rather than branching on the implementation class, {@link #harden(XMLInputFactory)} consolidates the JDK Zephyr and Woodstox recipes into one pass that
+ * probes which properties each factory accepts and adapts:</p>
+ * <ul>
+ *     <li><strong>Limits</strong>: applied best-effort by {@link Limits#tryApply(XMLInputFactory)}, which sets both the JDK and the Woodstox limit properties;
+ *         each implementation honours its own and rejects the other's.</li>
+ *     <li><strong>External DTD subset</strong>: skipped via Zephyr's {@value #ZEPHYR_IGNORE_EXTERNAL_DTD} (best-effort), so a DOCTYPE-only document parses
+ *         without a fetch attempt instead of tripping the deny-all resolver below. Woodstox skips it through {@value #WSTX_DTD_RESOLVER} instead.</li>
+ *     <li><strong>External entities</strong>: denied through resolvers, leaving the standard {@code SUPPORT_DTD} / {@code IS_SUPPORTING_EXTERNAL_ENTITIES}
+ *         defaults untouched. Woodstox exposes fine-grained hooks, so when all three apply the factory is Woodstox: {@value #WSTX_DTD_RESOLVER} (empty external
+ *         subset, but a thrown error on external parameter entities, which share that hook), {@value #WSTX_ENTITY_RESOLVER} (throw on declared external general
+ *         entities) and {@value #WSTX_UNDECLARED_ENTITY_RESOLVER} (silently drop undeclared references left by the skipped subset). Any factory that does not
+ *         accept that trio (the JDK Zephyr, or an unrecognized implementation) instead gets a single deny-all {@link Resolvers.DenyAll#XML} through
+ *         {@code setXMLResolver}.</li>
+ * </ul>
+ */
+final class StaxHardener {
+
+    /** Zephyr property: skip external DTD subset loading entirely, so a DOCTYPE-only document parses without a fetch attempt. */
+    private static final String ZEPHYR_IGNORE_EXTERNAL_DTD = "http://java.sun.com/xml/stream/properties/ignore-external-dtd";
+
+    /** Woodstox property: resolver consulted for the external DTD subset. */
+    private static final String WSTX_DTD_RESOLVER = "com.ctc.wstx.dtdResolver";
+
+    /** Woodstox property: resolver consulted for declared external general entities. */
+    private static final String WSTX_ENTITY_RESOLVER = "com.ctc.wstx.entityResolver";
+
+    /** Woodstox property: resolver consulted for undeclared entity references. */
+    private static final String WSTX_UNDECLARED_ENTITY_RESOLVER = "com.ctc.wstx.undeclaredEntityResolver";
+
+    /**
+     * Hybrid Woodstox DTD resolver: returns the empty input for the external DTD subset, throws on external parameter entities.
+     *
+     * <p>Woodstox calls this hook with {@code entityName == null} for the subset and {@code entityName != null} for parameter-entity expansion; that
+     * discriminator is Woodstox-specific (the JDK Zephyr's {@code XMLResolver} always receives {@code null} as the 4th argument), so the resolver lives
+     * here and is applied best-effort, ignored by implementations that do not recognize the property.</p>
+     */
+    static final XMLResolver DTD_SUBSET_ONLY = (publicID, systemID, baseURI, entityName) -> {
+        if (entityName != null) {
+            throw new XMLStreamException("External parameter entity '" + entityName + "' refused (publicID=" + publicID + ", systemID=" + systemID
+                    + ", baseURI=" + baseURI + ")");
+        }
+        return Resolvers.IgnoreAll.XML.resolveEntity(publicID, systemID, baseURI, entityName);
+    };
+
+    static XMLInputFactory harden(final XMLInputFactory factory) {
+        // Optional, implementation-based: JDK limit properties or Woodstox limit properties.
+        Limits.tryApply(factory);
+        // Optional: Zephyr's StAX equivalent of XERCES_LOAD_EXTERNAL_DTD=false skips the external DTD subset entirely.
+        setOptionalProperty(factory, ZEPHYR_IGNORE_EXTERNAL_DTD, true);
+
+        // Woodstox-specific fine-grained resolvers
+        if (!(trySetProperty(factory, WSTX_DTD_RESOLVER, DTD_SUBSET_ONLY)
+                && trySetProperty(factory, WSTX_ENTITY_RESOLVER, Resolvers.DenyAll.XML)
+                && trySetProperty(factory, WSTX_UNDECLARED_ENTITY_RESOLVER, Resolvers.IgnoreAll.XML))) {
+            // Fallback: use deny-all resolver
+            factory.setXMLResolver(Resolvers.DenyAll.XML);
+        }
+        return factory;
+    }
+
+    private StaxHardener() {
+    }
+}