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/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..f314f8546 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,72 @@ private static boolean isClassLoadable(String className) { } } + /** 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 + * Constants#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(Constants.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 + * 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 @@ -827,16 +889,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, + Constants.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..c664db0a2 100644 --- a/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java +++ b/api/src/test/java/javax/jdo/identity/ObjectIdentityTest.java @@ -31,12 +31,15 @@ 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; 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 +65,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( + Constants.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(Constants.PROPERTY_ALLOWED_IDENTITY_KEY_CLASSES); + } + @Override @Test void testConstructor() { @@ -425,6 +444,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(Constants.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 +527,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 { 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";