8180024: Improve construction of objects during deserialization

Reviewed-by: rriggs, skoivu, ahgross, rhalade
openjdk · May 19, 2017 · 020204a · 020204a
1 parent d36d599
commit 020204a
Show file tree

Hide file tree

Showing 4 changed files with 270 additions and 6 deletions.
diff --git a/src/java.base/share/classes/java/io/ObjectStreamClass.java b/src/java.base/share/classes/java/io/ObjectStreamClass.java
@@ -32,14 +32,19 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.UndeclaredThrowableException;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Proxy;
+import java.security.AccessControlContext;
 import java.security.AccessController;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.security.PermissionCollection;
+import java.security.Permissions;
 import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -53,7 +58,8 @@
 import jdk.internal.reflect.Reflection;
 import jdk.internal.reflect.ReflectionFactory;
 import sun.reflect.misc.ReflectUtil;
-
+import jdk.internal.misc.SharedSecrets;
+import jdk.internal.misc.JavaSecurityAccess;
 import static java.io.ObjectStreamField.*;
 
 /**
@@ -176,6 +182,9 @@ InvalidClassException newInvalidClassException() {
 
     /** serialization-appropriate constructor, or null if none */
     private Constructor<?> cons;
+    /** protection domains that need to be checked when calling the constructor */
+    private ProtectionDomain[] domains;
+
     /** class-defined writeObject method, or null if none */
     private Method writeObjectMethod;
     /** class-defined readObject method, or null if none */
@@ -508,6 +517,7 @@ public Void run() {
                             cl, "readObjectNoData", null, Void.TYPE);
                         hasWriteObjectData = (writeObjectMethod != null);
                     }
+                    domains = getProtectionDomains(cons, cl);
                     writeReplaceMethod = getInheritableMethod(
                         cl, "writeReplace", null, Object.class);
                     readResolveMethod = getInheritableMethod(
@@ -550,6 +560,65 @@ public Void run() {
     ObjectStreamClass() {
     }
 
+    /**
+     * Creates a PermissionDomain that grants no permission.
+     */
+    private ProtectionDomain noPermissionsDomain() {
+        PermissionCollection perms = new Permissions();
+        perms.setReadOnly();
+        return new ProtectionDomain(null, perms);
+    }
+
+    /**
+     * Aggregate the ProtectionDomains of all the classes that separate
+     * a concrete class {@code cl} from its ancestor's class declaring
+     * a constructor {@code cons}.
+     *
+     * If {@code cl} is defined by the boot loader, or the constructor
+     * {@code cons} is declared by {@code cl}, or if there is no security
+     * manager, then this method does nothing and {@code null} is returned.
+     *
+     * @param cons A constructor declared by {@code cl} or one of its
+     *             ancestors.
+     * @param cl A concrete class, which is either the class declaring
+     *           the constructor {@code cons}, or a serializable subclass
+     *           of that class.
+     * @return An array of ProtectionDomain representing the set of
+     *         ProtectionDomain that separate the concrete class {@code cl}
+     *         from its ancestor's declaring {@code cons}, or {@code null}.
+     */
+    private ProtectionDomain[] getProtectionDomains(Constructor<?> cons,
+                                                    Class<?> cl) {
+        ProtectionDomain[] domains = null;
+        if (cons != null && cl.getClassLoader() != null
+                && System.getSecurityManager() != null) {
+            Class<?> cls = cl;
+            Class<?> fnscl = cons.getDeclaringClass();
+            Set<ProtectionDomain> pds = null;
+            while (cls != fnscl) {
+                ProtectionDomain pd = cls.getProtectionDomain();
+                if (pd != null) {
+                    if (pds == null) pds = new HashSet<>();
+                    pds.add(pd);
+                }
+                cls = cls.getSuperclass();
+                if (cls == null) {
+                    // that's not supposed to happen
+                    // make a ProtectionDomain with no permission.
+                    // should we throw instead?
+                    if (pds == null) pds = new HashSet<>();
+                    else pds.clear();
+                    pds.add(noPermissionsDomain());
+                    break;
+                }
+            }
+            if (pds != null) {
+                domains = pds.toArray(new ProtectionDomain[0]);
+            }
+        }
+        return domains;
+    }
+
     /**
      * Initializes class descriptor representing a proxy class.
      */
@@ -580,6 +649,7 @@ void initProxy(Class<?> cl,
             writeReplaceMethod = localDesc.writeReplaceMethod;
             readResolveMethod = localDesc.readResolveMethod;
             deserializeEx = localDesc.deserializeEx;
+            domains = localDesc.domains;
             cons = localDesc.cons;
         }
         fieldRefl = getReflector(fields, localDesc);
@@ -666,6 +736,7 @@ void initNonProxy(ObjectStreamClass model,
             if (deserializeEx == null) {
                 deserializeEx = localDesc.deserializeEx;
             }
+            domains = localDesc.domains;
             cons = localDesc.cons;
         }
 
@@ -1006,7 +1077,35 @@ Object newInstance()
         requireInitialized();
         if (cons != null) {
             try {
-                return cons.newInstance();
+                if (domains == null || domains.length == 0) {
+                    return cons.newInstance();
+                } else {
+                    JavaSecurityAccess jsa = SharedSecrets.getJavaSecurityAccess();
+                    PrivilegedAction<?> pea = () -> {
+                        try {
+                            return cons.newInstance();
+                        } catch (InstantiationException
+                                 | InvocationTargetException
+                                 | IllegalAccessException x) {
+                            throw new UndeclaredThrowableException(x);
+                        }
+                    }; // Can't use PrivilegedExceptionAction with jsa
+                    try {
+                        return jsa.doIntersectionPrivilege(pea,
+                                   AccessController.getContext(),
+                                   new AccessControlContext(domains));
+                    } catch (UndeclaredThrowableException x) {
+                        Throwable cause = x.getCause();
+                        if (cause instanceof InstantiationException)
+                            throw (InstantiationException) cause;
+                        if (cause instanceof InvocationTargetException)
+                            throw (InvocationTargetException) cause;
+                        if (cause instanceof IllegalAccessException)
+                            throw (IllegalAccessException) cause;
+                        // not supposed to happen
+                        throw x;
+                    }
+                }
             } catch (IllegalAccessException ex) {
                 // should not occur, as access checks have been suppressed
                 throw new InternalError(ex);

diff --git a/src/java.corba/share/classes/com/sun/corba/se/impl/io/ObjectStreamClass.java b/src/java.corba/share/classes/com/sun/corba/se/impl/io/ObjectStreamClass.java
@@ -38,7 +38,10 @@
 import java.security.NoSuchAlgorithmException;
 import java.security.DigestOutputStream;
 import java.security.AccessController;
+import java.security.PermissionCollection;
+import java.security.Permissions;
 import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
 
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Field;
@@ -57,6 +60,8 @@
 
 import java.util.Arrays;
 import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Set;
 
 import com.sun.corba.se.impl.util.RepositoryId;
 
@@ -443,6 +448,65 @@ private static ObjectStreamField[] translateFields(java.io.ObjectStreamField[] f
     private static final PersistentFieldsValue persistentFieldsValue =
         new PersistentFieldsValue();
 
+    /**
+     * Creates a PermissionDomain that grants no permission.
+     */
+    private ProtectionDomain noPermissionsDomain() {
+        PermissionCollection perms = new Permissions();
+        perms.setReadOnly();
+        return new ProtectionDomain(null, perms);
+    }
+
+    /**
+     * Aggregate the ProtectionDomains of all the classes that separate
+     * a concrete class {@code cl} from its ancestor's class declaring
+     * a constructor {@code cons}.
+     *
+     * If {@code cl} is defined by the boot loader, or the constructor
+     * {@code cons} is declared by {@code cl}, or if there is no security
+     * manager, then this method does nothing and {@code null} is returned.
+     *
+     * @param cons A constructor declared by {@code cl} or one of its
+     *             ancestors.
+     * @param cl A concrete class, which is either the class declaring
+     *           the constructor {@code cons}, or a serializable subclass
+     *           of that class.
+     * @return An array of ProtectionDomain representing the set of
+     *         ProtectionDomain that separate the concrete class {@code cl}
+     *         from its ancestor's declaring {@code cons}, or {@code null}.
+     */
+    private ProtectionDomain[] getProtectionDomains(Constructor<?> cons,
+                                                    Class<?> cl) {
+        ProtectionDomain[] domains = null;
+        if (cons != null && cl.getClassLoader() != null
+                && System.getSecurityManager() != null) {
+            Class<?> cls = cl;
+            Class<?> fnscl = cons.getDeclaringClass();
+            Set<ProtectionDomain> pds = null;
+            while (cls != fnscl) {
+                ProtectionDomain pd = cls.getProtectionDomain();
+                if (pd != null) {
+                    if (pds == null) pds = new HashSet<>();
+                    pds.add(pd);
+                }
+                cls = cls.getSuperclass();
+                if (cls == null) {
+                    // that's not supposed to happen
+                    // make a ProtectionDomain with no permission.
+                    // should we throw instead?
+                    if (pds == null) pds = new HashSet<>();
+                    else pds.clear();
+                    pds.add(noPermissionsDomain());
+                    break;
+                }
+            }
+            if (pds != null) {
+                domains = pds.toArray(new ProtectionDomain[0]);
+            }
+        }
+        return domains;
+    }
+
     /*
      * Initialize class descriptor.  This method is only invoked on class
      * descriptors created via calls to lookupInternal().  This method is kept
@@ -568,11 +632,15 @@ public Object run() {
 
                 readResolveObjectMethod = bridge.readResolveForSerialization(cl);
 
+                domains = new ProtectionDomain[] {noPermissionsDomain()};
+
                 if (externalizable)
                     cons = getExternalizableConstructor(cl) ;
                 else
                     cons = getSerializableConstructor(cl) ;
 
+                domains = getProtectionDomains(cons, cl);
+
                 if (serializable && !forProxyClass) {
                     writeObjectMethod = bridge.writeObjectForSerialization(cl) ;
                     readObjectMethod = bridge.readObjectForSerialization(cl);
@@ -910,7 +978,7 @@ Object newInstance()
     {
         if (cons != null) {
             try {
-                return cons.newInstance();
+                return bridge.newInstanceForSerialization(cons, domains);
             } catch (IllegalAccessException ex) {
                 // should not occur, as access checks have been suppressed
                 InternalError ie = new InternalError();
@@ -1506,6 +1574,7 @@ public Object run() {
     private transient MethodHandle writeReplaceObjectMethod;
     private transient MethodHandle readResolveObjectMethod;
     private transient Constructor<?> cons;
+    private transient ProtectionDomain[] domains;
 
     /**
      * Beginning in Java to IDL ptc/02-01-12, RMI-IIOP has a

diff --git a/src/java.corba/share/classes/sun/corba/Bridge.java b/src/java.corba/share/classes/sun/corba/Bridge.java
@@ -27,8 +27,9 @@
 
 import java.io.OptionalDataException;
 import java.lang.invoke.MethodHandle;
-import java.lang.reflect.Field ;
-import java.lang.reflect.Constructor ;
+import java.lang.reflect.Field;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.StackWalker;
 import java.lang.StackWalker.StackFrame;
 import java.util.Optional;
@@ -37,6 +38,7 @@
 import java.security.AccessController;
 import java.security.Permission;
 import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
 
 import sun.misc.Unsafe;
 import sun.reflect.ReflectionFactory;
@@ -340,6 +342,36 @@ public final Constructor<?> newConstructorForExternalization(Class<?> cl) {
         return reflectionFactory.newConstructorForExternalization( cl ) ;
     }
 
+    /**
+     * Invokes the supplied constructor, adding the provided protection domains
+     * to the invocation stack before invoking {@code Constructor::newInstance}.
+     *
+     * This is equivalent to calling
+     * {@code ReflectionFactory.newInstanceForSerialization(cons,domains)}.
+     *
+     * @param cons A constructor obtained from {@code
+     *        newConstructorForSerialization} or {@code
+     *        newConstructorForExternalization}.
+     *
+     * @param domains An array of protection domains that limit the privileges
+     *        with which the constructor is invoked. Can be {@code null}
+     *        or empty, in which case privileges are only limited by the
+     *        {@linkplain AccessController#getContext() current context}.
+     *
+     * @return A new object built from the provided constructor.
+     *
+     * @throws NullPointerException if {@code cons} is {@code null}.
+     * @throws InstantiationException if thrown by {@code cons.newInstance()}.
+     * @throws InvocationTargetException if thrown by {@code cons.newInstance()}.
+     * @throws IllegalAccessException if thrown by {@code cons.newInstance()}.
+     */
+    public final Object newInstanceForSerialization(Constructor<?> cons,
+                                                    ProtectionDomain[] domains)
+        throws InstantiationException, InvocationTargetException, IllegalAccessException
+    {
+        return reflectionFactory.newInstanceForSerialization(cons, domains);
+    }
+
     /**
      * Returns true if the given class defines a static initializer method,
      * false otherwise.