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 aa1d0601..a36cc7a6 100644 --- a/api/src/main/java/javax/jdo/JDOHelper.java +++ b/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 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; } diff --git a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java index 10fd95c4..798d0cd6 100644 --- a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java +++ b/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(). * + *

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 +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 + * 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"); + } +} 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";