Document builder factory rehardened

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

@@ -1042,4 +1042,17 @@ public interface Constants {
    * @since 2.2
    */
   public static final String TX_SERIALIZABLE = "serializable";
+
+  /**
+   * The name of the boolean system property that, when set to "true", disables the re-application
+   * of the secure XML parsing defaults to a DocumentBuilderFactory registered via {@link
+   * javax.jdo.spi.JDOImplHelper#registerDocumentBuilderFactory}. By default a registered factory is
+   * hardened before each use exactly like the default factory (DOCTYPE declarations disallowed,
+   * entity references not expanded), so that registering a factory cannot silently re-enable
+   * external entity processing (XXE) during jdoconfig.xml parsing.
+   *
+   * @since 3.3
+   */
+  static final String PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY =
+      "javax.jdo.allowUnsafeDocumentBuilderFactory"; // NOI18N
 }

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

@@ -748,7 +748,7 @@
           continue;
         }
         // else assume first line of text is the PMF class name
         String[] tokens = line.split("\\s");
         String pmfClassName = tokens[0];
         int indexOfComment = pmfClassName.indexOf("#");
         if (indexOfComment == -1) {
@@ -1138,6 +1138,18 @@
     DocumentBuilderFactory factory = IMPL_HELPER.getRegisteredDocumentBuilderFactory();
     if (factory == null) {
       factory = getDefaultDocumentBuilderFactory();
+    } else if (!Boolean.getBoolean(Constants.PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY)) {
+      // 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

@@ -604,10 +604,22 @@ public static void registerAuthorizedStateManagerClasses(Collection<?> smClasses
    * 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, unless the system property
+   * <code>javax.jdo.allowUnsafeDocumentBuilderFactory</code> is set to "true". 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 +635,20 @@ public static DocumentBuilderFactory getRegisteredDocumentBuilderFactory() {
 
   /**
    * 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,87 @@
+/*
+ * 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);
+    System.clearProperty(JDOHelper.PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY);
+  }
+
+  /** 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");
+  }
+
+  /** The documented opt-out restores the previous behavior. */
+  @Test
+  void testRegisteredFactoryOptOut() throws ParserConfigurationException {
+    System.setProperty(JDOHelper.PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY, "true");
+    DocumentBuilderFactory unhardened = DocumentBuilderFactory.newInstance();
+    JDOImplHelper.getInstance().registerDocumentBuilderFactory(unhardened);
+
+    DocumentBuilderFactory factory = JDOHelper.getDocumentBuilderFactory();
+    Assertions.assertSame(unhardened, factory, "The registered factory must be preferred");
+    Assertions.assertFalse(
+        factory.getFeature(DISALLOW_DOCTYPE_DECL),
+        "With the opt-out property set, the registered factory must not be modified");
+  }
+}

tck/src/main/resources/conf/jdo-signatures.txt

@@ -254,6 +254,8 @@ public interface javax.jdo.Constants {
         = "javax/jdo/jdoquery_3_0.xsd";
     static String ANONYMOUS_PERSISTENCE_MANAGER_FACTORY_NAME
         = "";
+    static final String PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY
+        = "javax.jdo.allowUnsafeDocumentBuilderFactory";
     public static final String TX_READ_UNCOMMITTED = "read-uncommitted"; 
     public static final String TX_READ_COMMITTED = "read-committed"; 
     public static final String TX_REPEATABLE_READ = "repeatable-read";