Document builder factory revisited

api/src/main/java/javax/jdo/JDOHelper.java

@@ -1138,6 +1138,18 @@ protected static DocumentBuilderFactory getDocumentBuilderFactory() {
     DocumentBuilderFactory factory = IMPL_HELPER.getRegisteredDocumentBuilderFactory();
     if (factory == null) {
       factory = getDefaultDocumentBuilderFactory();
+    } else {
+      // Re-apply the secure defaults to the registered factory before every parse.
+      // Registration is an SPI open to any code in the process; without this, a factory
+      // registered with default settings would re-enable DOCTYPE processing (external
+      // entities / XXE) for every jdoconfig.xml on the classpath.
+      try {
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+      } catch (ParserConfigurationException e) {
+        // fail closed: do not parse with a factory that cannot disable DOCTYPEs
+        throw new JDOFatalUserException(e.getMessage());
+      }
+      factory.setExpandEntityReferences(false);
     }
     return factory;
   }

api/src/main/java/javax/jdo/spi/JDOImplHelper.java

@@ -162,7 +162,7 @@
     Set<String> mixedCased = createUserConfigurableStandardProperties();
     Set<String> lowerCased = new HashSet<>(mixedCased.size());
 
     for (String propertyName : mixedCased) {
       lowerCased.add(propertyName.toLowerCase());
     }
     return Collections.unmodifiableSet(lowerCased);
@@ -604,10 +604,21 @@
    * META-INF/jdoconfig.xml. The default is governed by the semantics of
    * DocumentBuilderFactory.newInstance().
    *
+   * <p>Note: secure XML parsing defaults (DOCTYPE declarations disallowed, entity references not
+   * expanded) are re-applied to the registered factory before each use. When running with a legacy
+   * SecurityManager, the caller must be authorized for <code>
+   * JDOPermission("manageMetadata")</code>.
+   *
    * @param factory the DocumentBuilderFactory instance to use
+   * @throws SecurityException if the caller is not authorized for JDOPermission("manageMetadata").
    * @since 2.1
    */
   public synchronized void registerDocumentBuilderFactory(DocumentBuilderFactory factory) {
+    SecurityManager sec = LegacyJava.getSecurityManager();
+    if (sec != null) {
+      // throws exception if caller is not authorized
+      sec.checkPermission(JDOPermission.MANAGE_METADATA);
+    }
     documentBuilderFactory = factory;
   }
 
@@ -623,12 +634,20 @@
 
   /**
    * Register an ErrorHandler instance for use in parsing the resource(s) META-INF/jdoconfig.xml.
-   * The default is an ErrorHandler that throws on error or fatalError and ignores warnings.
+   * The default is an ErrorHandler that throws on error or fatalError and ignores warnings. When
+   * running with a legacy SecurityManager, the caller must be authorized for <code>
+   * JDOPermission("manageMetadata")</code>.
    *
    * @param handler the ErrorHandler instance to use
+   * @throws SecurityException if the caller is not authorized for JDOPermission("manageMetadata").
    * @since 2.1
    */
   public synchronized void registerErrorHandler(ErrorHandler handler) {
+    SecurityManager sec = LegacyJava.getSecurityManager();
+    if (sec != null) {
+      // throws exception if caller is not authorized
+      sec.checkPermission(JDOPermission.MANAGE_METADATA);
+    }
     errorHandler = handler;
   }
 

api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java

@@ -0,0 +1,72 @@
+/*
+ * 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 javax.jdo;
+
+import javax.jdo.spi.JDOImplHelper;
+import javax.jdo.util.AbstractTest;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests that JDOHelper re-applies the secure XML parsing defaults to a DocumentBuilderFactory
+ * registered via JDOImplHelper before it is used for jdoconfig.xml parsing.
+ */
+class JDOHelperDocumentBuilderFactoryTest extends AbstractTest {
+
+  private static final String DISALLOW_DOCTYPE_DECL =
+      "http://apache.org/xml/features/disallow-doctype-decl";
+
+  @AfterEach
+  void cleanup() {
+    JDOImplHelper.getInstance().registerDocumentBuilderFactory(null);
+  }
+
+  /** The default factory is hardened. */
+  @Test
+  void testDefaultFactoryIsHardened() throws ParserConfigurationException {
+    DocumentBuilderFactory factory = JDOHelper.getDocumentBuilderFactory();
+    Assertions.assertTrue(
+        factory.getFeature(DISALLOW_DOCTYPE_DECL),
+        "Default DocumentBuilderFactory must disallow DOCTYPE declarations");
+    Assertions.assertFalse(
+        factory.isExpandEntityReferences(),
+        "Default DocumentBuilderFactory must not expand entity references");
+  }
+
+  /** A registered, unhardened factory is re-hardened before use. */
+  @Test
+  void testRegisteredFactoryIsRehardened() throws ParserConfigurationException {
+    DocumentBuilderFactory unhardened = DocumentBuilderFactory.newInstance();
+    Assertions.assertFalse(
+        unhardened.getFeature(DISALLOW_DOCTYPE_DECL),
+        "Precondition: a factory from newInstance() allows DOCTYPE declarations");
+    JDOImplHelper.getInstance().registerDocumentBuilderFactory(unhardened);
+
+    DocumentBuilderFactory factory = JDOHelper.getDocumentBuilderFactory();
+    Assertions.assertSame(unhardened, factory, "The registered factory must be preferred");
+    Assertions.assertTrue(
+        factory.getFeature(DISALLOW_DOCTYPE_DECL),
+        "The registered DocumentBuilderFactory must have DOCTYPE declarations re-disabled");
+    Assertions.assertFalse(
+        factory.isExpandEntityReferences(),
+        "The registered DocumentBuilderFactory must not expand entity references");
+  }
+}