Object identity construction

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 "<code>.*</code>", or "<code>*</code>" to disable the restriction
+   * entirely (restoring the previous, unrestricted behavior).
+   *
+   * <p>By default only classes with a registered {@link
+   * javax.jdo.spi.JDOImplHelper.StringConstructor}, classes in <code> java.lang</code>, <code>
+   * java.math</code> and <code>java.time</code>, and a small set of <code> java.util</code> 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
 }

api/src/main/java/javax/jdo/identity/ObjectIdentity.java

@@ -64,6 +64,12 @@ private static <T> T doPrivileged(PrivilegedAction<T> privilegedAction) {
   /**
    * Constructor with class and key.
    *
+   * <p>If <code>param</code> is a <code>String</code>, it is parsed as
+   * "&lt;className&gt;:&lt;keyString&gt;" 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
    */

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 @@
     }
   }
 
+  /** 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<String> 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.
+   *
+   * <p>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 @@
   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) {

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 "<className>:<keyString>".
+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 \

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 <T> void validateNestedException(JDOUserException ex, Class<T> 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 {

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";