8032948: Nashorn linkages awry
authorsundar
Thu, 30 Jan 2014 19:28:40 +0530
changeset 22668 245094625886
parent 22667 280928d0d6c8
child 22669 75563515567f
8032948: Nashorn linkages awry Reviewed-by: jlaskey, attila, ahgross
nashorn/src/jdk/nashorn/internal/objects/NativeObject.java
nashorn/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java
nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java
nashorn/src/jdk/nashorn/internal/runtime/linker/NashornStaticClassLinker.java
nashorn/src/jdk/nashorn/internal/runtime/linker/ReflectionCheckLinker.java
nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java
nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java	Thu Jan 30 18:49:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java	Thu Jan 30 19:28:40 2014 +0530
@@ -645,12 +645,12 @@
             targetObj.addBoundProperties(source, props);
         } else if (source instanceof StaticClass) {
             final Class<?> clazz = ((StaticClass)source).getRepresentedClass();
-            Bootstrap.checkReflectionAccess(clazz);
+            Bootstrap.checkReflectionAccess(clazz, true);
             bindBeanProperties(targetObj, source, BeansLinker.getReadableStaticPropertyNames(clazz),
                     BeansLinker.getWritableStaticPropertyNames(clazz), BeansLinker.getStaticMethodNames(clazz));
         } else {
             final Class<?> clazz = source.getClass();
-            Bootstrap.checkReflectionAccess(clazz);
+            Bootstrap.checkReflectionAccess(clazz, false);
             bindBeanProperties(targetObj, source, BeansLinker.getReadableInstancePropertyNames(clazz),
                     BeansLinker.getWritableInstancePropertyNames(clazz), BeansLinker.getInstanceMethodNames(clazz));
         }
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java	Thu Jan 30 18:49:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java	Thu Jan 30 19:28:40 2014 +0530
@@ -278,9 +278,10 @@
      * {@code java.lang.invoke} package, as well a {@link Class} and any subclass of {@link ClassLoader}) and there is
      * a security manager in the system, then it checks the {@code nashorn.JavaReflection} {@code RuntimePermission}.
      * @param clazz the class being tested
+     * @param isStatic is access checked for static members (or instance members)
      */
-    public static void checkReflectionAccess(Class<?> clazz) {
-        ReflectionCheckLinker.checkReflectionAccess(clazz);
+    public static void checkReflectionAccess(Class<?> clazz, boolean isStatic) {
+        ReflectionCheckLinker.checkReflectionAccess(clazz, isStatic);
     }
 
     /**
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java	Thu Jan 30 18:49:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java	Thu Jan 30 19:28:40 2014 +0530
@@ -111,7 +111,7 @@
                 // check for restricted package access
                 Context.checkPackageAccess(type);
                 // check for classes, interfaces in reflection
-                ReflectionCheckLinker.checkReflectionAccess(type);
+                ReflectionCheckLinker.checkReflectionAccess(type, true);
             }
         }
         return getAdapterInfo(types).getAdapterClassFor(classOverrides);
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornStaticClassLinker.java	Thu Jan 30 18:49:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornStaticClassLinker.java	Thu Jan 30 19:28:40 2014 +0530
@@ -65,7 +65,7 @@
             return null;
         }
         final Class<?> receiverClass = ((StaticClass) self).getRepresentedClass();
-        Bootstrap.checkReflectionAccess(receiverClass);
+        Bootstrap.checkReflectionAccess(receiverClass, true);
         final CallSiteDescriptor desc = request.getCallSiteDescriptor();
         // We intercept "new" on StaticClass instances to provide additional capabilities
         if ("new".equals(desc.getNameToken(CallSiteDescriptor.OPERATOR))) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/ReflectionCheckLinker.java	Thu Jan 30 18:49:47 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/ReflectionCheckLinker.java	Thu Jan 30 19:28:40 2014 +0530
@@ -26,6 +26,7 @@
 package jdk.nashorn.internal.runtime.linker;
 
 import java.lang.reflect.Modifier;
+import java.lang.reflect.Proxy;
 import jdk.internal.dynalink.CallSiteDescriptor;
 import jdk.internal.dynalink.linker.GuardedInvocation;
 import jdk.internal.dynalink.linker.LinkRequest;
@@ -47,6 +48,7 @@
         if (type == Class.class || ClassLoader.class.isAssignableFrom(type)) {
             return true;
         }
+
         final String name = type.getName();
         return name.startsWith("java.lang.reflect.") || name.startsWith("java.lang.invoke.");
     }
@@ -59,9 +61,25 @@
         return null;
     }
 
-    static void checkReflectionAccess(Class<?> clazz) {
+    private static boolean isReflectiveCheckNeeded(final Class<?> type, final boolean isStatic) {
+         // special handling for Proxy subclasses
+         if (Proxy.class.isAssignableFrom(type)) {
+            if (Proxy.isProxyClass(type)) {
+                // real Proxy class - filter only static access
+                return isStatic;
+            }
+
+            // fake Proxy subclass - filter it always!
+            return true;
+        }
+
+        // check for any other reflective Class
+        return isReflectionClass(type);
+    }
+
+    static void checkReflectionAccess(final Class<?> clazz, final boolean isStatic) {
         final SecurityManager sm = System.getSecurityManager();
-        if (sm != null && isReflectionClass(clazz)) {
+        if (sm != null && isReflectiveCheckNeeded(clazz, isStatic)) {
             checkReflectionPermission(sm);
         }
     }
@@ -78,6 +96,7 @@
                     if (desc.getNameTokenCount() > CallSiteDescriptor.NAME_OPERAND &&
                         "static".equals(desc.getNameToken(CallSiteDescriptor.NAME_OPERAND))) {
                         if (Context.isAccessibleClass((Class<?>)self) && !isReflectionClass((Class<?>)self)) {
+
                             // If "getProp:static" passes access checks, allow access.
                             return;
                         }
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java	Thu Jan 30 18:49:47 2014 +0530
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java	Thu Jan 30 19:28:40 2014 +0530
@@ -27,6 +27,9 @@
 
 import static org.testng.Assert.fail;
 
+import java.lang.reflect.Method;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Proxy;
 import java.util.Objects;
 import javax.script.Invocable;
 import javax.script.ScriptEngine;
@@ -183,4 +186,98 @@
             }
         }
     }
+
+    // @bug 8032948: Nashorn linkages awry
+    public static class FakeProxy extends Proxy {
+        public FakeProxy(InvocationHandler ih) {
+            super(ih);
+        }
+
+        public static Class<?> makeProxyClass(ClassLoader cl, Class<?>... ifaces) {
+            return Proxy.getProxyClass(cl, ifaces);
+        }
+    }
+
+    @Test
+    public void fakeProxySubclassAccessCheckTest() throws ScriptException {
+        if (System.getSecurityManager() == null) {
+            // pass vacuously
+            return;
+        }
+
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        e.put("name", ScriptEngineSecurityTest.class.getName());
+        e.put("cl", ScriptEngineSecurityTest.class.getClassLoader());
+        e.put("intfs", new Class[] { Runnable.class });
+
+        String getClass = "Java.type(name + '$FakeProxy').getProxyClass(cl, intfs);";
+
+        // Should not be able to call static methods of Proxy via fake subclass
+        try {
+            Class c = (Class)e.eval(getClass);
+            fail("should have thrown SecurityException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof SecurityException)) {
+                fail("SecurityException expected, got " + exp);
+            }
+        }
+    }
+
+    @Test
+    public void fakeProxySubclassAccessCheckTest2() throws ScriptException {
+        if (System.getSecurityManager() == null) {
+            // pass vacuously
+            return;
+        }
+
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        e.put("name", ScriptEngineSecurityTest.class.getName());
+        e.put("cl", ScriptEngineSecurityTest.class.getClassLoader());
+        e.put("intfs", new Class[] { Runnable.class });
+
+        String getClass = "Java.type(name + '$FakeProxy').makeProxyClass(cl, intfs);";
+
+        // Should not be able to call static methods of Proxy via fake subclass
+        try {
+            Class c = (Class)e.eval(getClass);
+            fail("should have thrown SecurityException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof SecurityException)) {
+                fail("SecurityException expected, got " + exp);
+            }
+        }
+    }
+
+    @Test
+    public static void proxyStaticAccessCheckTest() throws ScriptException {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+        final Runnable r = (Runnable)Proxy.newProxyInstance(
+            ScriptEngineTest.class.getClassLoader(),
+            new Class[] { Runnable.class },
+            new InvocationHandler() {
+                @Override
+                public Object invoke(Object p, Method m, Object[] a) {
+                    return null;
+                }
+            });
+
+        e.put("rc", r.getClass());
+        e.put("cl", ScriptEngineSecurityTest.class.getClassLoader());
+        e.put("intfs", new Class[] { Runnable.class });
+
+        // make sure static methods of Proxy is not accessible via subclass
+        try {
+            e.eval("rc.static.getProxyClass(cl, intfs)");
+            fail("Should have thrown SecurityException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof SecurityException)) {
+                fail("SecurityException expected, got " + exp);
+            }
+        }
+    }
 }
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Thu Jan 30 18:49:47 2014 +0530
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Thu Jan 30 19:28:40 2014 +0530
@@ -33,7 +33,9 @@
 import java.io.PrintWriter;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 import java.util.concurrent.Callable;
 import javax.script.Compilable;
 import javax.script.CompiledScript;
@@ -535,6 +537,29 @@
         assertEquals(e.eval("Window.funcJSObject(obj)"), "hello");
     }
 
+    // @bug 8032948: Nashorn linkages awry
+    @Test
+    public void checkProxyAccess() throws ScriptException {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+        final boolean[] reached = new boolean[1];
+        final Runnable r = (Runnable)Proxy.newProxyInstance(
+            ScriptEngineTest.class.getClassLoader(),
+            new Class[] { Runnable.class },
+            new InvocationHandler() {
+                @Override
+                public Object invoke(Object p, Method m, Object[] a) {
+                    reached[0] = true;
+                    return null;
+                }
+            });
+
+        e.put("r", r);
+        e.eval("r.run()");
+
+        assertTrue(reached[0]);
+    }
+
     private static final String LINE_SEPARATOR = System.getProperty("line.separator");
 
     // Returns String that would be the result of calling PrintWriter.println