From ccaaf01be6926d260059be0d7698d5b672fcd983 Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Fri, 28 Aug 2026 21:39:16 +0200 Subject: [PATCH 1/6] Registered DocumentBuilderFactory re-hardened before every jdoconfig parse, fail-closed; registry gains MANAGE_METADATA gate --- api/src/main/java/javax/jdo/JDOHelper.java | 25 ++++++ .../java/javax/jdo/spi/JDOImplHelper.java | 24 ++++- .../JDOHelperDocumentBuilderFactoryTest.java | 87 +++++++++++++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java diff --git a/api/src/main/java/javax/jdo/JDOHelper.java b/api/src/main/java/javax/jdo/JDOHelper.java index aa1d0601..05524b6d 100644 --- a/api/src/main/java/javax/jdo/JDOHelper.java +++ b/api/src/main/java/javax/jdo/JDOHelper.java @@ -1133,11 +1133,36 @@ protected static Map getNamedPMFProperties( return propertiesByNameInAllConfigs.get(name); } + /** + * 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 + * 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 + */ + public static final String PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY = + "javax.jdo.allowUnsafeDocumentBuilderFactory"; // NOI18N + protected static DocumentBuilderFactory getDocumentBuilderFactory() { @SuppressWarnings("static-access") DocumentBuilderFactory factory = IMPL_HELPER.getRegisteredDocumentBuilderFactory(); if (factory == null) { factory = getDefaultDocumentBuilderFactory(); + } else if (!Boolean.getBoolean(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; } diff --git a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java index 10fd95c4..345df50c 100644 --- a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java +++ b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java @@ -604,10 +604,23 @@ public static void registerAuthorizedStateManagerClasses(Collection smClasses * META-INF/jdoconfig.xml. The default is governed by the semantics of * DocumentBuilderFactory.newInstance(). * + *

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 + * javax.jdo.allowUnsafeDocumentBuilderFactory is set to "true". When running with a + * legacy SecurityManager, the caller must be authorized for + * JDOPermission("manageMetadata"). + * * @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 +636,21 @@ 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 + * JDOPermission("manageMetadata"). * * @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; } diff --git a/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java b/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java new file mode 100644 index 00000000..2d7b453d --- /dev/null +++ b/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"); + } +} From 88788e0aa75e70d0f1f44a5e8d24117c04584341 Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Sun, 30 Aug 2026 18:32:55 +0200 Subject: [PATCH 2/6] Moved property name constant to Constant interface and fixed jdo-signature --- api/src/main/java/javax/jdo/Constants.java | 13 +++++++++++++ api/src/main/java/javax/jdo/JDOHelper.java | 15 +-------------- .../main/java/javax/jdo/spi/JDOImplHelper.java | 6 ++---- tck/src/main/resources/conf/jdo-signatures.txt | 2 ++ 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/api/src/main/java/javax/jdo/Constants.java b/api/src/main/java/javax/jdo/Constants.java index aed3c407..1e70b95a 100644 --- a/api/src/main/java/javax/jdo/Constants.java +++ b/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 } diff --git a/api/src/main/java/javax/jdo/JDOHelper.java b/api/src/main/java/javax/jdo/JDOHelper.java index 05524b6d..a36cc7a6 100644 --- a/api/src/main/java/javax/jdo/JDOHelper.java +++ b/api/src/main/java/javax/jdo/JDOHelper.java @@ -1133,25 +1133,12 @@ protected static Map getNamedPMFProperties( return propertiesByNameInAllConfigs.get(name); } - /** - * 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 - * 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 - */ - public static final String PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY = - "javax.jdo.allowUnsafeDocumentBuilderFactory"; // NOI18N - protected static DocumentBuilderFactory getDocumentBuilderFactory() { @SuppressWarnings("static-access") DocumentBuilderFactory factory = IMPL_HELPER.getRegisteredDocumentBuilderFactory(); if (factory == null) { factory = getDefaultDocumentBuilderFactory(); - } else if (!Boolean.getBoolean(PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY)) { + } 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 diff --git a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java index 345df50c..798d0cd6 100644 --- a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java +++ b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java @@ -611,8 +611,7 @@ public static void registerAuthorizedStateManagerClasses(Collection smClasses * JDOPermission("manageMetadata"). * * @param factory the DocumentBuilderFactory instance to use - * @throws SecurityException if the caller is not authorized for - * JDOPermission("manageMetadata"). + * @throws SecurityException if the caller is not authorized for JDOPermission("manageMetadata"). * @since 2.1 */ public synchronized void registerDocumentBuilderFactory(DocumentBuilderFactory factory) { @@ -641,8 +640,7 @@ public static DocumentBuilderFactory getRegisteredDocumentBuilderFactory() { * JDOPermission("manageMetadata"). * * @param handler the ErrorHandler instance to use - * @throws SecurityException if the caller is not authorized for - * JDOPermission("manageMetadata"). + * @throws SecurityException if the caller is not authorized for JDOPermission("manageMetadata"). * @since 2.1 */ public synchronized void registerErrorHandler(ErrorHandler handler) { diff --git a/tck/src/main/resources/conf/jdo-signatures.txt b/tck/src/main/resources/conf/jdo-signatures.txt index 01cc8ad7..aa72a892 100644 --- a/tck/src/main/resources/conf/jdo-signatures.txt +++ b/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"; From 079d5d7a07f35da5eb13c254fdec83d4afe4edf8 Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Sun, 27 Sep 2026 17:23:45 +0200 Subject: [PATCH 3/6] Removed Property javax.jdo.allowUnsafeDocumentBuilderFactory --- api/src/main/java/javax/jdo/Constants.java | 12 ------------ api/src/main/java/javax/jdo/JDOHelper.java | 2 +- .../jdo/JDOHelperDocumentBuilderFactoryTest.java | 14 -------------- tck/src/main/resources/conf/jdo-signatures.txt | 2 -- 4 files changed, 1 insertion(+), 29 deletions(-) diff --git a/api/src/main/java/javax/jdo/Constants.java b/api/src/main/java/javax/jdo/Constants.java index 1e70b95a..5f25a14e 100644 --- a/api/src/main/java/javax/jdo/Constants.java +++ b/api/src/main/java/javax/jdo/Constants.java @@ -1043,16 +1043,4 @@ public interface Constants { */ 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 } diff --git a/api/src/main/java/javax/jdo/JDOHelper.java b/api/src/main/java/javax/jdo/JDOHelper.java index a36cc7a6..f35a485f 100644 --- a/api/src/main/java/javax/jdo/JDOHelper.java +++ b/api/src/main/java/javax/jdo/JDOHelper.java @@ -1138,7 +1138,7 @@ protected static DocumentBuilderFactory getDocumentBuilderFactory() { DocumentBuilderFactory factory = IMPL_HELPER.getRegisteredDocumentBuilderFactory(); if (factory == null) { factory = getDefaultDocumentBuilderFactory(); - } else if (!Boolean.getBoolean(Constants.PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY)) { + } 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 diff --git a/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java b/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java index 2d7b453d..3ae4153b 100644 --- a/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java +++ b/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java @@ -37,7 +37,6 @@ class JDOHelperDocumentBuilderFactoryTest extends AbstractTest { @AfterEach void cleanup() { JDOImplHelper.getInstance().registerDocumentBuilderFactory(null); - System.clearProperty(JDOHelper.PROPERTY_ALLOW_UNSAFE_DOCUMENT_BUILDER_FACTORY); } /** The default factory is hardened. */ @@ -71,17 +70,4 @@ void testRegisteredFactoryIsRehardened() throws ParserConfigurationException { "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"); - } } diff --git a/tck/src/main/resources/conf/jdo-signatures.txt b/tck/src/main/resources/conf/jdo-signatures.txt index aa72a892..01cc8ad7 100644 --- a/tck/src/main/resources/conf/jdo-signatures.txt +++ b/tck/src/main/resources/conf/jdo-signatures.txt @@ -254,8 +254,6 @@ 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"; From 8608cea4e2f4b44e1a04c769fa50678829638c3d Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Sun, 27 Sep 2026 17:28:28 +0200 Subject: [PATCH 4/6] Formatting changes --- api/src/main/java/javax/jdo/Constants.java | 1 - .../test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java | 1 - 2 files changed, 2 deletions(-) diff --git a/api/src/main/java/javax/jdo/Constants.java b/api/src/main/java/javax/jdo/Constants.java index 5f25a14e..aed3c407 100644 --- a/api/src/main/java/javax/jdo/Constants.java +++ b/api/src/main/java/javax/jdo/Constants.java @@ -1042,5 +1042,4 @@ public interface Constants { * @since 2.2 */ public static final String TX_SERIALIZABLE = "serializable"; - } diff --git a/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java b/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java index 3ae4153b..96a1703b 100644 --- a/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java +++ b/api/src/test/java/javax/jdo/JDOHelperDocumentBuilderFactoryTest.java @@ -69,5 +69,4 @@ void testRegisteredFactoryIsRehardened() throws ParserConfigurationException { factory.isExpandEntityReferences(), "The registered DocumentBuilderFactory must not expand entity references"); } - } From 2cab2fe8d0e0c554772bab2f5ecb1c8b5e4c3dac Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Tue, 6 Oct 2026 20:49:31 +0200 Subject: [PATCH 5/6] Fixed comment --- api/src/main/java/javax/jdo/spi/JDOImplHelper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java index 798d0cd6..875203c1 100644 --- a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java +++ b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java @@ -605,8 +605,7 @@ public static void registerAuthorizedStateManagerClasses(Collection smClasses * DocumentBuilderFactory.newInstance(). * *

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 - * javax.jdo.allowUnsafeDocumentBuilderFactory is set to "true". When running with a + * expanded) are re-applied to the registered factory before each use. When running with a * legacy SecurityManager, the caller must be authorized for * JDOPermission("manageMetadata"). * From c1979bd673bdd04f4391063844c685afb74bc370 Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Tue, 6 Oct 2026 21:25:12 +0200 Subject: [PATCH 6/6] formatting changes --- api/src/main/java/javax/jdo/spi/JDOImplHelper.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java index 875203c1..95993cdf 100644 --- a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java +++ b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java @@ -605,8 +605,8 @@ public static void registerAuthorizedStateManagerClasses(Collection smClasses * DocumentBuilderFactory.newInstance(). * *

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 + * expanded) are re-applied to the registered factory before each use. When running with a legacy + * SecurityManager, the caller must be authorized for * JDOPermission("manageMetadata"). * * @param factory the DocumentBuilderFactory instance to use