From 71e0ff94cc29f9e44f7f2cc8dedea1a986b814a7 Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Fri, 28 Aug 2026 21:41:37 +0200 Subject: [PATCH 1/2] Identity-key construction gated on an allowlist; Class.forName no longer initializes unvetted classes --- .../javax/jdo/identity/ObjectIdentity.java | 6 ++ .../java/javax/jdo/spi/JDOImplHelper.java | 100 ++++++++++++++++-- .../resources/javax/jdo/Bundle.properties | 4 + .../jdo/identity/ObjectIdentityTest.java | 64 +++++++++++ 4 files changed, 167 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/javax/jdo/identity/ObjectIdentity.java b/api/src/main/java/javax/jdo/identity/ObjectIdentity.java index ca8cd0a8e..6019d1b0f 100644 --- a/api/src/main/java/javax/jdo/identity/ObjectIdentity.java +++ b/api/src/main/java/javax/jdo/identity/ObjectIdentity.java @@ -64,6 +64,12 @@ private static T doPrivileged(PrivilegedAction privilegedAction) { /** * Constructor with class and key. * + *

If param is a String, it is parsed as + * "<className>:<keyString>" and the key instance is constructed by {@link + * JDOImplHelper#construct(String, String)}. Because the String form may originate from an + * untrusted source, only allowed identity key classes may be named by it; see {@link + * JDOImplHelper#PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES}. + * * @param pcClass the class * @param param the key */ diff --git a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java index 10fd95c40..8f8fa8b11 100644 --- a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java +++ b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java @@ -29,6 +29,7 @@ import java.text.ParsePosition; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Currency; @@ -814,11 +815,89 @@ private static boolean isClassLoadable(String className) { } } + /** + * The name of the system property used to extend the set of key classes that {@link + * #construct(String, String)} may instantiate from a String. The value is a comma-separated list + * of entries; an entry is either a fully-qualified class name, a package prefix ending in + * ".*", or "*" to disable the restriction entirely (restoring the + * previous, unrestricted behavior). + * + *

By default only classes with a registered {@link StringConstructor}, classes in + * java.lang, java.math and java.time, and a small set of + * java.util value classes may be constructed. This prevents the String form of an + * identity (see {@link javax.jdo.identity.ObjectIdentity}), which may originate from an + * untrusted source, from loading and instantiating arbitrary classes. + * + * @since 3.3 + */ + public static final String PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES = + "javax.jdo.allowedIdentityKeyClasses"; // NOI18N + + /** Package prefixes of key classes always allowed for String construction. */ + private static final String[] DEFAULT_ALLOWED_KEY_CLASS_PREFIXES = { + "java.lang.", "java.math.", "java.time." // NOI18N + }; + + /** Individual key classes always allowed for String construction. */ + private static final Set DEFAULT_ALLOWED_KEY_CLASS_NAMES = + new HashSet<>( + Arrays.asList( + "java.util.Date", // NOI18N + "java.util.Locale", // NOI18N + "java.util.Currency", // NOI18N + "java.util.UUID")); // NOI18N + + /** + * Determine whether the given class may be instantiated from a String by {@link + * #construct(String, String)}. A class is allowed if it belongs to the built-in set of value + * classes or is named by the system property {@link #PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES}. + * + * @param keyClass the candidate key class + * @return true if the class may be constructed from a String + */ + private static boolean isIdentityKeyClassAllowed(Class keyClass) { + String name = keyClass.getName(); + for (String prefix : DEFAULT_ALLOWED_KEY_CLASS_PREFIXES) { + if (name.startsWith(prefix)) { + return true; + } + } + if (DEFAULT_ALLOWED_KEY_CLASS_NAMES.contains(name)) { + return true; + } + String allowed = System.getProperty(PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES); + if (allowed != null) { + for (String rawEntry : allowed.split(",")) { // NOI18N + String entry = rawEntry.trim(); + if (entry.isEmpty()) { + continue; + } + if ("*".equals(entry)) { // NOI18N + return true; + } + if (entry.endsWith(".*")) { // NOI18N + if (name.startsWith(entry.substring(0, entry.length() - 1))) { + return true; + } + } else if (entry.equals(name)) { + return true; + } + } + } + return false; + } + /** * Construct an instance of the parameter class, using the keyString as an argument to the - * constructor. If the class has a StringConstructor instance registered, use it. If not, try to - * find a constructor for the class with a single String argument. Otherwise, throw a - * JDOUserException. + * constructor. If the class has a StringConstructor instance registered, use it. If not, and the + * class is an allowed identity key class, try to find a constructor for the class with a single + * String argument. Otherwise, throw a JDOUserException. + * + *

Because the class name typically originates from the String form of an identity, which may + * come from an untrusted source, only classes with a registered {@link StringConstructor}, the + * built-in value classes, or classes named by the system property {@link + * #PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES} are instantiated. The named class is loaded without + * initializing it, so no code of a disallowed class runs. * * @param className the name of the class * @param keyString the String parameter for the constructor @@ -827,16 +906,23 @@ private static boolean isClassLoadable(String className) { public static Object construct(String className, String keyString) { StringConstructor stringConstructor; try { - Class keyClass = Class.forName(className); + // load without initializing: no static initializer of an unvetted class may run + Class keyClass = Class.forName(className, false, JDOImplHelper.class.getClassLoader()); synchronized (stringConstructorMap) { stringConstructor = stringConstructorMap.get(keyClass); } if (stringConstructor != null) { return stringConstructor.construct(keyString); - } else { - Constructor keyConstructor = keyClass.getConstructor(String.class); - return keyConstructor.newInstance(keyString); } + if (!isIdentityKeyClassAllowed(keyClass)) { + throw new JDOUserException( + msg.msg( + "EXC_ObjectIdentityStringConstructionKeyClassNotAllowed", // NOI18N + className, + PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES)); + } + Constructor keyConstructor = keyClass.getConstructor(String.class); + return keyConstructor.newInstance(keyString); } catch (JDOException ex) { throw ex; } catch (Exception ex) { diff --git a/api/src/main/resources/javax/jdo/Bundle.properties b/api/src/main/resources/javax/jdo/Bundle.properties index 208ef4561..2354354ef 100644 --- a/api/src/main/resources/javax/jdo/Bundle.properties +++ b/api/src/main/resources/javax/jdo/Bundle.properties @@ -73,6 +73,10 @@ EXC_ObjectIdentityStringConstructionTooShort: Parameter is too short. EXC_ObjectIdentityStringConstructionUsage: The instance could not be constructed \ from the parameter String "{0}". \ \nThe parameter String is of the form ":". +EXC_ObjectIdentityStringConstructionKeyClassNotAllowed: The class "{0}" is not an \ +allowed identity key class, so no instance was constructed from the String form \ +of the identity. Register a JDOImplHelper.StringConstructor for the class, or add \ +the class name to the system property "{1}". EXC_CreateKeyAsObjectMustNotBeCalled: The method createKeyAsObject must not be called \ because the keyAsObject field must never be null for this class. EXC_CurrencyStringConstructorIllegalArgument: The instance could not be constructed \ diff --git a/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java b/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java index 05e7bae0a..2f892be4b 100644 --- a/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java +++ b/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java @@ -36,7 +36,9 @@ import javax.jdo.JDOUserException; import javax.jdo.LegacyJava; import javax.jdo.spi.JDOImplHelper; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; /** */ @@ -62,6 +64,22 @@ public ObjectIdentityTest() { // This method body is intentionally left blank } + /** Allow the test key classes by exact name, so that other classes stay disallowed. */ + @BeforeAll + static void setAllowedIdentityKeyClasses() { + System.setProperty( + JDOImplHelper.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES, + "javax.jdo.identity.ObjectIdentityTest$IdClass," + + "javax.jdo.identity.ObjectIdentityTest$BadIdClassNoStringConstructor," + + "javax.jdo.identity.ObjectIdentityTest$BadIdClassNoPublicStringConstructor"); + } + + /** Restore the default allowlist. */ + @AfterAll + static void clearAllowedIdentityKeyClasses() { + System.clearProperty(JDOImplHelper.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES); + } + @Override @Test void testConstructor() { @@ -425,6 +443,38 @@ void testGetKeyAsObject() { Assertions.assertEquals(c1.getKeyAsObject(), new IdClass(1), "keyAsObject doesn't match."); } + @Test + void testStringConstructorDefaultAllowedKeyClass() { + // java.math.* is in the built-in allowlist; no system property entry needed + ObjectIdentity c1 = new ObjectIdentity(Object.class, "java.math.BigDecimal:123.45"); + Assertions.assertEquals(new BigDecimal("123.45"), c1.getKeyAsObject()); + } + + @Test + void testStringConstructorDisallowedKeyClass() { + // java.io.File has a public (String) constructor but is not an allowed key class + JDOUserException ex = + Assertions.assertThrows( + JDOUserException.class, + () -> new ObjectIdentity(Object.class, "java.io.File:/tmp/x"), + "Failed to catch expected JDOUserException for disallowed key class."); + Assertions.assertTrue( + ex.getMessage().contains(JDOImplHelper.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES), + "Exception should name the allowlist system property: " + ex.getMessage()); + } + + @Test + void testStringConstructorDisallowedKeyClassNotInitialized() { + Assertions.assertThrows( + JDOUserException.class, + () -> + new ObjectIdentity( + Object.class, "javax.jdo.identity.ObjectIdentityTest$StaticInitCanary:x"), + "Failed to catch expected JDOUserException for disallowed key class."); + Assertions.assertFalse( + canaryStaticInitRun, "Static initializer of a disallowed key class must not run."); + } + private void validateNestedException(JDOUserException ex, Class expected) { Throwable[] nesteds = ex.getNestedExceptions(); if (nesteds == null || nesteds.length != 1) { @@ -476,6 +526,20 @@ public boolean equals(Object obj) { } } + /** Set by StaticInitCanary's static initializer; must remain false. */ + static boolean canaryStaticInitRun = false; + + /** Not in the allowlist; its static initializer must never run via ObjectIdentity. */ + public static class StaticInitCanary { + static { + canaryStaticInitRun = true; + } + + public StaticInitCanary(String str) { + // This method body is intentionally left blank + } + } + public static class BadIdClassNoStringConstructor {} public static class BadIdClassNoPublicStringConstructor { From 33bcd6eeda2f82caca4fa922c459dc83d9d3e01e Mon Sep 17 00:00:00 2001 From: Michael Bouschen Date: Sun, 30 Aug 2026 18:16:28 +0200 Subject: [PATCH 2/2] Moved property name constant to Constant interface and fixed jdo-signature. --- api/src/main/java/javax/jdo/Constants.java | 19 ++++++++++++ .../java/javax/jdo/spi/JDOImplHelper.java | 29 ++++--------------- .../jdo/identity/ObjectIdentityTest.java | 7 +++-- .../main/resources/conf/jdo-signatures.txt | 2 ++ 4 files changed, 31 insertions(+), 26 deletions(-) diff --git a/api/src/main/java/javax/jdo/Constants.java b/api/src/main/java/javax/jdo/Constants.java index aed3c4079..05fb884ba 100644 --- a/api/src/main/java/javax/jdo/Constants.java +++ b/api/src/main/java/javax/jdo/Constants.java @@ -1042,4 +1042,23 @@ public interface Constants { * @since 2.2 */ public static final String TX_SERIALIZABLE = "serializable"; + + /** + * The name of the system property used to extend the set of key classes that {@link + * javax.jdo.spi.JDOImplHelper#construct(String, String)} may instantiate from a String. The value + * is a comma-separated list of entries; an entry is either a fully-qualified class name, a + * package prefix ending in ".*", or "*" to disable the restriction + * entirely (restoring the previous, unrestricted behavior). + * + *

By default only classes with a registered {@link + * javax.jdo.spi.JDOImplHelper.StringConstructor}, classes in java.lang, + * java.math and java.time, and a small set of java.util value + * classes may be constructed. This prevents the String form of an identity (see {@link + * javax.jdo.identity.ObjectIdentity}), which may originate from an untrusted source, from loading + * and instantiating arbitrary classes. + * + * @since 3.3 + */ + static final String PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES = + "javax.jdo.allowedIdentityKeyClasses"; // NOI18N } diff --git a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java index 8f8fa8b11..f314f8546 100644 --- a/api/src/main/java/javax/jdo/spi/JDOImplHelper.java +++ b/api/src/main/java/javax/jdo/spi/JDOImplHelper.java @@ -815,24 +815,6 @@ private static boolean isClassLoadable(String className) { } } - /** - * The name of the system property used to extend the set of key classes that {@link - * #construct(String, String)} may instantiate from a String. The value is a comma-separated list - * of entries; an entry is either a fully-qualified class name, a package prefix ending in - * ".*", or "*" to disable the restriction entirely (restoring the - * previous, unrestricted behavior). - * - *

By default only classes with a registered {@link StringConstructor}, classes in - * java.lang, java.math and java.time, and a small set of - * java.util value classes may be constructed. This prevents the String form of an - * identity (see {@link javax.jdo.identity.ObjectIdentity}), which may originate from an - * untrusted source, from loading and instantiating arbitrary classes. - * - * @since 3.3 - */ - public static final String PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES = - "javax.jdo.allowedIdentityKeyClasses"; // NOI18N - /** Package prefixes of key classes always allowed for String construction. */ private static final String[] DEFAULT_ALLOWED_KEY_CLASS_PREFIXES = { "java.lang.", "java.math.", "java.time." // NOI18N @@ -850,7 +832,8 @@ private static boolean isClassLoadable(String className) { /** * Determine whether the given class may be instantiated from a String by {@link * #construct(String, String)}. A class is allowed if it belongs to the built-in set of value - * classes or is named by the system property {@link #PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES}. + * classes or is named by the system property {@link + * Constants#PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES}. * * @param keyClass the candidate key class * @return true if the class may be constructed from a String @@ -865,7 +848,7 @@ private static boolean isIdentityKeyClassAllowed(Class keyClass) { if (DEFAULT_ALLOWED_KEY_CLASS_NAMES.contains(name)) { return true; } - String allowed = System.getProperty(PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES); + String allowed = System.getProperty(Constants.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES); if (allowed != null) { for (String rawEntry : allowed.split(",")) { // NOI18N String entry = rawEntry.trim(); @@ -896,8 +879,8 @@ private static boolean isIdentityKeyClassAllowed(Class keyClass) { *

Because the class name typically originates from the String form of an identity, which may * come from an untrusted source, only classes with a registered {@link StringConstructor}, the * built-in value classes, or classes named by the system property {@link - * #PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES} are instantiated. The named class is loaded without - * initializing it, so no code of a disallowed class runs. + * Constants#PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES} are instantiated. The named class is loaded + * without initializing it, so no code of a disallowed class runs. * * @param className the name of the class * @param keyString the String parameter for the constructor @@ -919,7 +902,7 @@ public static Object construct(String className, String keyString) { msg.msg( "EXC_ObjectIdentityStringConstructionKeyClassNotAllowed", // NOI18N className, - PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES)); + Constants.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES)); } Constructor keyConstructor = keyClass.getConstructor(String.class); return keyConstructor.newInstance(keyString); diff --git a/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java b/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java index 2f892be4b..c664db0a2 100644 --- a/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java +++ b/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java @@ -31,6 +31,7 @@ import java.util.Currency; import java.util.Date; import java.util.Locale; +import javax.jdo.Constants; import javax.jdo.JDOFatalInternalException; import javax.jdo.JDONullIdentityException; import javax.jdo.JDOUserException; @@ -68,7 +69,7 @@ public ObjectIdentityTest() { @BeforeAll static void setAllowedIdentityKeyClasses() { System.setProperty( - JDOImplHelper.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES, + Constants.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES, "javax.jdo.identity.ObjectIdentityTest$IdClass," + "javax.jdo.identity.ObjectIdentityTest$BadIdClassNoStringConstructor," + "javax.jdo.identity.ObjectIdentityTest$BadIdClassNoPublicStringConstructor"); @@ -77,7 +78,7 @@ static void setAllowedIdentityKeyClasses() { /** Restore the default allowlist. */ @AfterAll static void clearAllowedIdentityKeyClasses() { - System.clearProperty(JDOImplHelper.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES); + System.clearProperty(Constants.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES); } @Override @@ -459,7 +460,7 @@ void testStringConstructorDisallowedKeyClass() { () -> new ObjectIdentity(Object.class, "java.io.File:/tmp/x"), "Failed to catch expected JDOUserException for disallowed key class."); Assertions.assertTrue( - ex.getMessage().contains(JDOImplHelper.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES), + ex.getMessage().contains(Constants.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES), "Exception should name the allowlist system property: " + ex.getMessage()); } diff --git a/tck/src/main/resources/conf/jdo-signatures.txt b/tck/src/main/resources/conf/jdo-signatures.txt index 01cc8ad77..b1712f322 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_ALLOWED_IDENTITY_KEY_CLASSES + = "javax.jdo.allowedIdentityKeyClasses"; 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";