8017196: Ensure Proxies are handled appropriately
authormchung
Mon, 22 Jul 2013 19:41:07 -0700
changeset 20825 3d5429b4b601
parent 20824 5f75136da7c7
child 20826 d9ee83c801f6
8017196: Ensure Proxies are handled appropriately Reviewed-by: dfuchs, jrose, jdn, ahgross, chegar
jdk/src/share/classes/java/lang/invoke/MethodHandles.java
jdk/src/share/classes/java/lang/reflect/Proxy.java
jdk/src/share/classes/sun/reflect/misc/ReflectUtil.java
--- a/jdk/src/share/classes/java/lang/invoke/MethodHandles.java	Mon Jul 22 14:02:38 2013 +0100
+++ b/jdk/src/share/classes/java/lang/invoke/MethodHandles.java	Mon Jul 22 19:41:07 2013 -0700
@@ -433,7 +433,7 @@
         Lookup(Class<?> lookupClass) {
             this(lookupClass, ALL_MODES);
             // make sure we haven't accidentally picked up a privileged class:
-            checkUnprivilegedlookupClass(lookupClass);
+            checkUnprivilegedlookupClass(lookupClass, ALL_MODES);
         }
 
         private Lookup(Class<?> lookupClass, int allowedModes) {
@@ -487,7 +487,7 @@
                 // No permissions.
                 newModes = 0;
             }
-            checkUnprivilegedlookupClass(requestedLookupClass);
+            checkUnprivilegedlookupClass(requestedLookupClass, newModes);
             return new Lookup(requestedLookupClass, newModes);
         }
 
@@ -503,10 +503,19 @@
         /** Package-private version of lookup which is trusted. */
         static final Lookup IMPL_LOOKUP = new Lookup(Object.class, TRUSTED);
 
-        private static void checkUnprivilegedlookupClass(Class<?> lookupClass) {
+        private static void checkUnprivilegedlookupClass(Class<?> lookupClass, int allowedModes) {
             String name = lookupClass.getName();
             if (name.startsWith("java.lang.invoke."))
                 throw newIllegalArgumentException("illegal lookupClass: "+lookupClass);
+
+            // For caller-sensitive MethodHandles.lookup()
+            // disallow lookup more restricted packages
+            if (allowedModes == ALL_MODES && lookupClass.getClassLoader() == null) {
+                if (name.startsWith("java.") ||
+                        (name.startsWith("sun.") && !name.startsWith("sun.invoke."))) {
+                    throw newIllegalArgumentException("illegal lookupClass: " + lookupClass);
+                }
+            }
         }
 
         /**
--- a/jdk/src/share/classes/java/lang/reflect/Proxy.java	Mon Jul 22 14:02:38 2013 +0100
+++ b/jdk/src/share/classes/java/lang/reflect/Proxy.java	Mon Jul 22 19:41:07 2013 -0700
@@ -347,11 +347,11 @@
      *             s.checkPermission} with
      *             {@code RuntimePermission("getClassLoader")} permission
      *             denies access.</li>
-     *             <li> the caller's class loader is not the same as or an
-     *             ancestor of the class loader for the current class and
+     *             <li> for each proxy interface, {@code intf},
+     *             the caller's class loader is not the same as or an
+     *             ancestor of the class loader for {@code intf} and
      *             invocation of {@link SecurityManager#checkPackageAccess
-     *             s.checkPackageAccess()} denies access to any one of the
-     *             given proxy interfaces.</li>
+     *             s.checkPackageAccess()} denies access to {@code intf}.</li>
      *          </ul>
 
      * @throws  NullPointerException if the {@code interfaces} array
@@ -680,11 +680,11 @@
      *               s.checkPermission} with
      *               {@code RuntimePermission("getClassLoader")} permission
      *               denies access;</li>
-     *          <li> the caller's class loader is not the same as or an
-     *               ancestor of the class loader for the current class and
+     *          <li> for each proxy interface, {@code intf},
+     *               the caller's class loader is not the same as or an
+     *               ancestor of the class loader for {@code intf} and
      *               invocation of {@link SecurityManager#checkPackageAccess
-     *               s.checkPackageAccess()} denies access to any one of the
-     *               given proxy interfaces.</li>
+     *               s.checkPackageAccess()} denies access to {@code intf};</li>
      *          <li> any of the given proxy interfaces is non-public and the
      *               caller class is not in the same {@linkplain Package runtime package}
      *               as the non-public interface and the invocation of
@@ -795,7 +795,14 @@
      * @return  the invocation handler for the proxy instance
      * @throws  IllegalArgumentException if the argument is not a
      *          proxy instance
+     * @throws  SecurityException if a security manager, <em>s</em>, is present
+     *          and the caller's class loader is not the same as or an
+     *          ancestor of the class loader for the invocation handler
+     *          and invocation of {@link SecurityManager#checkPackageAccess
+     *          s.checkPackageAccess()} denies access to the invocation
+     *          handler's class.
      */
+    @CallerSensitive
     public static InvocationHandler getInvocationHandler(Object proxy)
         throws IllegalArgumentException
     {
@@ -806,8 +813,19 @@
             throw new IllegalArgumentException("not a proxy instance");
         }
 
-        Proxy p = (Proxy) proxy;
-        return p.h;
+        final Proxy p = (Proxy) proxy;
+        final InvocationHandler ih = p.h;
+        if (System.getSecurityManager() != null) {
+            Class<?> ihClass = ih.getClass();
+            Class<?> caller = Reflection.getCallerClass();
+            if (ReflectUtil.needsPackageAccessCheck(caller.getClassLoader(),
+                                                    ihClass.getClassLoader()))
+            {
+                ReflectUtil.checkPackageAccess(ihClass);
+            }
+        }
+
+        return ih;
     }
 
     private static native Class<?> defineClass0(ClassLoader loader, String name,
--- a/jdk/src/share/classes/sun/reflect/misc/ReflectUtil.java	Mon Jul 22 14:02:38 2013 +0100
+++ b/jdk/src/share/classes/sun/reflect/misc/ReflectUtil.java	Mon Jul 22 19:41:07 2013 -0700
@@ -26,8 +26,10 @@
 
 package sun.reflect.misc;
 
+import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Proxy;
+import java.util.Arrays;
 import sun.reflect.Reflection;
 
 public final class ReflectUtil {
@@ -250,4 +252,50 @@
         String pkg = (i != -1) ? name.substring(0, i) : "";
         return Proxy.isProxyClass(cls) && !pkg.equals(PROXY_PACKAGE);
     }
+
+    /**
+     * Check if the given method is a method declared in the proxy interface
+     * implemented by the given proxy instance.
+     *
+     * @param proxy a proxy instance
+     * @param method an interface method dispatched to a InvocationHandler
+     *
+     * @throws IllegalArgumentException if the given proxy or method is invalid.
+     */
+    public static void checkProxyMethod(Object proxy, Method method) {
+        // check if it is a valid proxy instance
+        if (proxy == null || !Proxy.isProxyClass(proxy.getClass())) {
+            throw new IllegalArgumentException("Not a Proxy instance");
 }
+        if (Modifier.isStatic(method.getModifiers())) {
+            throw new IllegalArgumentException("Can't handle static method");
+        }
+
+        Class<?> c = method.getDeclaringClass();
+        if (c == Object.class) {
+            String name = method.getName();
+            if (name.equals("hashCode") || name.equals("equals") || name.equals("toString")) {
+                return;
+            }
+        }
+
+        if (isSuperInterface(proxy.getClass(), c)) {
+            return;
+        }
+
+        // disallow any method not declared in one of the proxy intefaces
+        throw new IllegalArgumentException("Can't handle: " + method);
+    }
+
+    private static boolean isSuperInterface(Class<?> c, Class<?> intf) {
+        for (Class<?> i : c.getInterfaces()) {
+            if (i == intf) {
+                return true;
+            }
+            if (isSuperInterface(i, intf)) {
+                return true;
+            }
+        }
+        return false;
+    }
+}