8021262: Make nashorn access checks consistent with underlying dynalink

Reviewed-by: jlaskey, lagergren, attila

nashorn/make/code_coverage.xml

@@ -132,7 +132,7 @@
       <arg value="-exclude"/>
       <arg value="com\.oracle\.nashorn\.runtime\.ScriptRuntime*"/>
       <arg value="-exclude"/>
-      <arg value="jdk\.nashorn\.internal\.javaadapters*"/>
+      <arg value="jdk\.nashorn\.javaadapters*"/>
       <arg value="-exclude"/>
       <arg value="jdk\.nashorn\.internal\.objects\.annotations*"/>
       <arg value="-exclude"/>

nashorn/src/jdk/nashorn/internal/codegen/SpillObjectCreator.java

@@ -65,7 +65,7 @@ public class SpillObjectCreator extends ObjectCreator {
 
         final int      length       = keys.size();
         final Object[] presetValues = new Object[propertyMap.size()];
-        final Class    clazz        = JO.class;
+        final Class<?> clazz        = JO.class;
 
         // Compute constant values
         for (int i = 0; i < length; i++) {

nashorn/src/jdk/nashorn/internal/objects/NativeDate.java

@@ -44,7 +44,6 @@ import jdk.nashorn.internal.runtime.ConsString;
 import jdk.nashorn.internal.runtime.JSType;
 import jdk.nashorn.internal.runtime.PropertyMap;
 import jdk.nashorn.internal.runtime.ScriptEnvironment;
-import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.linker.Bootstrap;

nashorn/src/jdk/nashorn/internal/objects/NativeObject.java

@@ -55,7 +55,6 @@ import jdk.nashorn.internal.runtime.ECMAException;
 import jdk.nashorn.internal.runtime.JSType;
 import jdk.nashorn.internal.runtime.Property;
 import jdk.nashorn.internal.runtime.PropertyMap;
-import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.linker.Bootstrap;

nashorn/src/jdk/nashorn/internal/runtime/Context.java

@@ -40,10 +40,13 @@ import java.lang.reflect.Modifier;
 import java.util.concurrent.atomic.AtomicLong;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.security.AccessControlContext;
 import java.security.AccessController;
 import java.security.CodeSigner;
 import java.security.CodeSource;
+import java.security.Permissions;
 import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
 import java.util.Map;
 import jdk.internal.org.objectweb.asm.ClassReader;
 import jdk.internal.org.objectweb.asm.util.CheckClassAdapter;
@@ -201,6 +204,7 @@ public final class Context {
 
     private static final ClassLoader myLoader = Context.class.getClassLoader();
     private static final StructureLoader sharedLoader;
+    private static final AccessControlContext NO_PERMISSIONS_CONTEXT;
 
     static {
         sharedLoader = AccessController.doPrivileged(new PrivilegedAction<StructureLoader>() {
@@ -209,6 +213,7 @@ public final class Context {
                 return new StructureLoader(myLoader, null);
             }
         });
+        NO_PERMISSIONS_CONTEXT = new AccessControlContext(new ProtectionDomain[] { new ProtectionDomain(null, new Permissions()) });
     }
 
     /**
@@ -479,7 +484,7 @@ public final class Context {
                 source = new Source(name, script);
             }
         } else if (src instanceof Map) {
-            final Map map = (Map)src;
+            final Map<?,?> map = (Map<?,?>)src;
             if (map.containsKey("script") && map.containsKey("name")) {
                 final String script = JSType.toString(map.get("script"));
                 final String name   = JSType.toString(map.get("name"));
@@ -549,11 +554,14 @@ public final class Context {
      * @throws ClassNotFoundException if structure class cannot be resolved
      */
     public static Class<?> forStructureClass(final String fullName) throws ClassNotFoundException {
+        if (System.getSecurityManager() != null && !NashornLoader.isStructureClass(fullName)) {
+            throw new ClassNotFoundException(fullName);
+        }
         return Class.forName(fullName, true, sharedLoader);
     }
 
     /**
-     * Checks that the given package can be accessed from current call stack.
+     * Checks that the given package can be accessed from no permissions context.
      *
      * @param fullName fully qualified package name
      * @throw SecurityException if not accessible
@@ -563,13 +571,19 @@ public final class Context {
         if (index != -1) {
             final SecurityManager sm = System.getSecurityManager();
             if (sm != null) {
-                sm.checkPackageAccess(fullName.substring(0, index));
+                AccessController.doPrivileged(new PrivilegedAction<Void>() {
+                    @Override
+                    public Void run() {
+                        sm.checkPackageAccess(fullName.substring(0, index));
+                        return null;
+                    }
+                }, NO_PERMISSIONS_CONTEXT);
             }
         }
     }
 
     /**
-     * Checks that the given package can be accessed from current call stack.
+     * Checks that the given package can be accessed from no permissions context.
      *
      * @param fullName fully qualified package name
      * @return true if package is accessible, false otherwise
@@ -584,7 +598,7 @@ public final class Context {
     }
 
     /**
-     * Checks that the given Class can be accessed from current call stack and is public.
+     * Checks that the given Class is public and it can be accessed from no permissions context.
      *
      * @param clazz Class object to check
      * @return true if Class is accessible, false otherwise

nashorn/src/jdk/nashorn/internal/runtime/NashornLoader.java

@@ -118,6 +118,10 @@ abstract class NashornLoader extends SecureClassLoader {
         return permCollection;
     }
 
+    static boolean isStructureClass(final String fullName) {
+        return fullName.startsWith(SCRIPTS_PKG);
+    }
+
     /**
      * Create a secure URL class loader for the given classpath
      * @param classPath classpath for the loader to search from

nashorn/src/jdk/nashorn/internal/runtime/PropertyMap.java

@@ -352,11 +352,15 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
         return newMap;
     }
 
-    /*
+    /**
      * Make a new UserAccessorProperty property. getter and setter functions are stored in
      * this ScriptObject and slot values are used in property object. Note that slots
      * are assigned speculatively and should be added to map before adding other
      * properties.
+     *
+     * @param key the property name
+     * @param propertyFlags attribute flags of the property
+     * @return the newly created UserAccessorProperty
      */
     public UserAccessorProperty newUserAccessors(final String key, final int propertyFlags) {
         int oldSpillLength = spillLength;

nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java

@@ -3192,9 +3192,15 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
         return true;
     }
 
-    /*
+    /**
      * Make a new UserAccessorProperty property. getter and setter functions are stored in
      * this ScriptObject and slot values are used in property object.
+     *
+     * @param key the property name
+     * @param propertyFlags attribute flags of the property
+     * @param getter getter function for the property
+     * @param setter setter function for the property
+     * @return the newly created UserAccessorProperty
      */
     protected final UserAccessorProperty newUserAccessors(final String key, final int propertyFlags, final ScriptFunction getter, final ScriptFunction setter) {
         final UserAccessorProperty property = getMap().newUserAccessors(key, propertyFlags);

nashorn/src/jdk/nashorn/internal/runtime/ScriptRuntime.java

@@ -391,7 +391,7 @@ public final class ScriptRuntime {
         return construct(target, args);
     }
 
-    /*
+    /**
      * Call a script function as a constructor with given args.
      *
      * @param target ScriptFunction object.

nashorn/src/jdk/nashorn/internal/runtime/Source.java

@@ -384,11 +384,7 @@ public final class Source {
         }
 
         final byte[] buf = Files.readAllBytes(file.toPath());
-        if (cs != null) {
+        return (cs != null)? new String(buf, cs).toCharArray() : byteToCharArray(buf);
-            return new String(buf, cs).toCharArray();
-        } else {
-            return byteToCharArray(buf);
-        }
     }
 
     /**
@@ -465,11 +461,7 @@ public final class Source {
     }
 
     private static char[] readFully(final InputStream is, final Charset cs) throws IOException {
-        if (cs != null) {
+        return (cs != null)? new String(readBytes(is), cs).toCharArray() : readFully(is);
-            return new String(readBytes(is), cs).toCharArray();
-        } else {
-            return readFully(is);
-        }
     }
 
     private static char[] readFully(final InputStream is) throws IOException {

nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java

@@ -51,7 +51,6 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
@@ -179,8 +178,6 @@ final class JavaAdapterBytecodeGenerator {
      */
     private static final Collection<MethodInfo> EXCLUDED = getExcludedMethods();
 
-    private static final Random random = new SecureRandom();
-
     // This is the superclass for our generated adapter.
     private final Class<?> superClass;
     // Class loader used as the parent for the class loader we'll create to load the generated class. It will be a class
@@ -230,12 +227,6 @@ final class JavaAdapterBytecodeGenerator {
         superClassName = Type.getInternalName(superClass);
         generatedClassName = getGeneratedClassName(superClass, interfaces);
 
-        // Randomize the name of the privileged global setter, to make it non-feasible to find.
-        final long l;
-        synchronized(random) {
-            l = random.nextLong();
-        }
-
         cw.visit(Opcodes.V1_7, ACC_PUBLIC | ACC_SUPER | ACC_FINAL, generatedClassName, null, superClassName, getInternalTypeNames(interfaces));
 
         generateGlobalFields();

nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterClassLoader.java

@@ -73,16 +73,6 @@ final class JavaAdapterClassLoader {
         });
     }
 
-    private static class AdapterLoader extends SecureClassLoader {
-        AdapterLoader(ClassLoader parent) {
-            super(parent);
-        }
-    }
-
-    static boolean isAdapterClass(Class<?> clazz) {
-        return clazz.getClassLoader() instanceof AdapterLoader;
-    }
-
     // Note that the adapter class is created in the protection domain of the class/interface being
     // extended/implemented, and only the privileged global setter action class is generated in the protection domain
     // of Nashorn itself. Also note that the creation and loading of the global setter is deferred until it is
@@ -91,7 +81,7 @@ final class JavaAdapterClassLoader {
     // with ability to introspect on the class and use setAccessible(true) on it could invoke the method. It's a
     // security tradeoff...
     private ClassLoader createClassLoader(final ClassLoader parentLoader) {
-        return new AdapterLoader(parentLoader) {
+        return new SecureClassLoader(parentLoader) {
             private final ClassLoader myLoader = getClass().getClassLoader();
 
             @Override

nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java

@@ -137,15 +137,6 @@ public final class JavaAdapterFactory {
         });
     }
 
-    /**
-     * Tells if the given Class is an adapter or support class
-     * @param clazz Class object
-     * @return true if the Class given is adapter or support class
-     */
-    public static boolean isAdapterClass(Class<?> clazz) {
-        return JavaAdapterClassLoader.isAdapterClass(clazz);
-    }
-
     /**
      * Returns whether an instance of the specified class/interface can be generated from a ScriptFunction. Returns true
      * iff: the adapter for the class/interface can be created, it is abstract (this includes interfaces), it has at

nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java

@@ -477,6 +477,7 @@ public class LinkerCallSite extends ChainedCallSite {
         /**
          * Tracer function that logs a callsite miss
          *
+         * @param desc callsite descriptor string
          * @param args arguments to function
          *
          * @throws Throwable if invocation failes or throws exception/error

nashorn/src/jdk/nashorn/internal/runtime/linker/NashornBottomLinker.java

@@ -169,31 +169,43 @@ final class NashornBottomLinker implements GuardingDynamicLinker {
         return ScriptRuntime.safeToString(linkRequest.getArguments()[1]);
     }
 
-    // Returns @FunctionalInterface annotated interface's single abstract method.
+    // cache of @FunctionalInterface method of implementor classes
-    // If not found, returns null
+    private static final ClassValue<Method> FUNCTIONAL_IFACE_METHOD = new ClassValue<Method>() {
-    static Method getFunctionalInterfaceMethod(final Class<?> clazz) {
+        @Override
-        if (clazz == null) {
+        protected Method computeValue(final Class<?> type) {
-            return null;
+            return findFunctionalInterfaceMethod(type);
         }
 
-        for (Class<?> iface : clazz.getInterfaces()) {
+        private Method findFunctionalInterfaceMethod(final Class<?> clazz) {
-            // check accessiblity up-front
+            if (clazz == null) {
-            if (! Context.isAccessibleClass(iface)) {
+                return null;
-                continue;
             }
 
-            // check for @FunctionalInterface
+            for (Class<?> iface : clazz.getInterfaces()) {
-            if (iface.isAnnotationPresent(FunctionalInterface.class)) {
+                // check accessiblity up-front
-                // return the first abstract method
+                if (! Context.isAccessibleClass(iface)) {
-                for (final Method m : iface.getMethods()) {
+                    continue;
-                    if (Modifier.isAbstract(m.getModifiers())) {
+                }
-                        return m;
+
+                // check for @FunctionalInterface
+                if (iface.isAnnotationPresent(FunctionalInterface.class)) {
+                    // return the first abstract method
+                    for (final Method m : iface.getMethods()) {
+                        if (Modifier.isAbstract(m.getModifiers())) {
+                            return m;
+                        }
                     }
                 }
             }
-        }
 
-        // did not find here, try super class
+            // did not find here, try super class
-        return getFunctionalInterfaceMethod(clazz.getSuperclass());
+            return findFunctionalInterfaceMethod(clazz.getSuperclass());
+        }
+    };
+
+    // Returns @FunctionalInterface annotated interface's single abstract
+    // method. If not found, returns null.
+    static Method getFunctionalInterfaceMethod(final Class<?> clazz) {
+        return FUNCTIONAL_IFACE_METHOD.get(clazz);
     }
 }

nashorn/src/jdk/nashorn/internal/runtime/linker/NashornStaticClassLinker.java

@@ -34,6 +34,7 @@ import jdk.internal.dynalink.linker.LinkRequest;
 import jdk.internal.dynalink.linker.LinkerServices;
 import jdk.internal.dynalink.linker.TypeBasedGuardingDynamicLinker;
 import jdk.internal.dynalink.support.Guards;
+import jdk.nashorn.internal.runtime.Context;
 import jdk.nashorn.internal.runtime.ECMAErrors;
 
 /**
@@ -68,6 +69,9 @@ final class NashornStaticClassLinker implements TypeBasedGuardingDynamicLinker {
         final CallSiteDescriptor desc = request.getCallSiteDescriptor();
         // We intercept "new" on StaticClass instances to provide additional capabilities
         if ("new".equals(desc.getNameToken(CallSiteDescriptor.OPERATOR))) {
+            // make sure new is on accessible Class
+            Context.checkPackageAccess(receiverClass.getName());
+
             // Is the class abstract? (This includes interfaces.)
             if (NashornLinker.isAbstractClass(receiverClass)) {
                 // Change this link request into a link request on the adapter class.

nashorn/src/jdk/nashorn/internal/runtime/linker/ReflectionCheckLinker.java

@@ -76,9 +76,10 @@ final class ReflectionCheckLinker implements TypeBasedGuardingDynamicLinker{
                 final CallSiteDescriptor desc = requestWithoutContext.getCallSiteDescriptor();
                 if(CallSiteDescriptorFactory.tokenizeOperators(desc).contains("getProp")) {
                     if ("static".equals(desc.getNameToken(CallSiteDescriptor.NAME_OPERAND))) {
-                        Context.checkPackageAccess(((Class)self).getName());
+                        if (Context.isAccessibleClass((Class<?>)self) && !isReflectionClass((Class<?>)self)) {
-                        // If "getProp:static" passes package access, allow access.
+                            // If "getProp:static" passes access checks, allow access.
-                        return;
+                            return;
+                        }
                     }
                 }
             }

nashorn/test/script/sandbox/nashorninternals.js

@@ -44,17 +44,25 @@ function checkClass(name) {
 
 // Not exhaustive - but a representative list of classes
 checkClass("jdk.nashorn.internal.codegen.Compiler");
-checkClass("jdk.nashorn.internal.codegen.objects.MapCreator");
 checkClass("jdk.nashorn.internal.codegen.types.Type");
 checkClass("jdk.nashorn.internal.ir.Node");
 checkClass("jdk.nashorn.internal.ir.FunctionNode");
 checkClass("jdk.nashorn.internal.ir.debug.JSONWriter");
 checkClass("jdk.nashorn.internal.ir.visitor.NodeVisitor");
+checkClass("jdk.nashorn.internal.lookup.MethodHandleFactory");
+checkClass("jdk.nashorn.internal.objects.Global");
 checkClass("jdk.nashorn.internal.parser.AbstractParser");
 checkClass("jdk.nashorn.internal.parser.Parser");
 checkClass("jdk.nashorn.internal.parser.JSONParser");
 checkClass("jdk.nashorn.internal.parser.Lexer");
 checkClass("jdk.nashorn.internal.parser.Scanner");
+checkClass("jdk.nashorn.internal.runtime.Context");
+checkClass("jdk.nashorn.internal.runtime.arrays.ArrayData");
+checkClass("jdk.nashorn.internal.runtime.linker.Bootstrap");
+checkClass("jdk.nashorn.internal.runtime.options.Option");
+checkClass("jdk.nashorn.internal.runtime.regexp.RegExp");
+checkClass("jdk.nashorn.internal.scripts.JO");
+checkClass("jdk.nashorn.tools.Shell");
 checkClass("jdk.internal.dynalink.CallSiteDescriptor");
 checkClass("jdk.internal.dynalink.beans.StaticClass");
 checkClass("jdk.internal.dynalink.linker.LinkRequest");

nashorn/test/script/trusted/JDK-8006529.js

@@ -39,20 +39,21 @@
  * and FunctionNode because of package-access check and so reflective calls.
  */
 
-var Parser              = Java.type("jdk.nashorn.internal.parser.Parser")
+var forName = java.lang.Class["forName(String)"];
-var Compiler            = Java.type("jdk.nashorn.internal.codegen.Compiler")
+var Parser              = forName("jdk.nashorn.internal.parser.Parser").static
-var Context             = Java.type("jdk.nashorn.internal.runtime.Context")
+var Compiler            = forName("jdk.nashorn.internal.codegen.Compiler").static
-var ScriptEnvironment   = Java.type("jdk.nashorn.internal.runtime.ScriptEnvironment")
+var Context             = forName("jdk.nashorn.internal.runtime.Context").static
-var Source              = Java.type("jdk.nashorn.internal.runtime.Source")
+var ScriptEnvironment   = forName("jdk.nashorn.internal.runtime.ScriptEnvironment").static
-var FunctionNode        = Java.type("jdk.nashorn.internal.ir.FunctionNode")
+var Source              = forName("jdk.nashorn.internal.runtime.Source").static
-var Block               = Java.type("jdk.nashorn.internal.ir.Block")
+var FunctionNode        = forName("jdk.nashorn.internal.ir.FunctionNode").static
-var VarNode             = Java.type("jdk.nashorn.internal.ir.VarNode")
+var Block               = forName("jdk.nashorn.internal.ir.Block").static
-var ExpressionStatement = Java.type("jdk.nashorn.internal.ir.ExpressionStatement")
+var VarNode             = forName("jdk.nashorn.internal.ir.VarNode").static
-var UnaryNode           = Java.type("jdk.nashorn.internal.ir.UnaryNode")
+var ExpressionStatement = forName("jdk.nashorn.internal.ir.ExpressionStatement").static
-var BinaryNode          = Java.type("jdk.nashorn.internal.ir.BinaryNode")
+var UnaryNode           = forName("jdk.nashorn.internal.ir.UnaryNode").static
-var ThrowErrorManager   = Java.type("jdk.nashorn.internal.runtime.Context$ThrowErrorManager")
+var BinaryNode          = forName("jdk.nashorn.internal.ir.BinaryNode").static
-var ErrorManager        = Java.type("jdk.nashorn.internal.runtime.ErrorManager")
+var ThrowErrorManager   = forName("jdk.nashorn.internal.runtime.Context$ThrowErrorManager").static
-var Debug               = Java.type("jdk.nashorn.internal.runtime.Debug")
+var ErrorManager        = forName("jdk.nashorn.internal.runtime.ErrorManager").static
+var Debug               = forName("jdk.nashorn.internal.runtime.Debug").static
 
 var parseMethod = Parser.class.getMethod("parse");
 var compileMethod = Compiler.class.getMethod("compile", FunctionNode.class);

nashorn/test/script/trusted/JDK-8021129.js

@@ -29,9 +29,9 @@
  * @test
  * @run
  */
-var R = Java.type("jdk.nashorn.internal.test.models.InternalRunnable")
-var r1 = R.class.newInstance()
 
+var InternalRunnableSuperclass = Java.type("jdk.nashorn.test.models.InternalRunnableSuperclass");
+var r1 = InternalRunnableSuperclass.makeInternalRunnable();
 r1.run() // Can execute method from an implemented non-restricted interface
 print(r1.toString()) // Can execute public method from a superclass
 
@@ -41,5 +41,5 @@ print(r1.invisibleProperty === undefined) // Can't see any other properties
 print(r1.canSeeThisField === undefined) // Can't see fields from superclasses
 print(r1.canNotSeeThisField === undefined) // Can't see its own fields
 
-var r2 = new (Java.type("jdk.nashorn.test.models.InternalRunnableSuperclass"))
+var r2 = new InternalRunnableSuperclass();
 print(r2.canSeeThisField) // Superclass field works fine on its own

nashorn/test/script/trusted/JDK-8021189.js

@@ -27,8 +27,9 @@
  * @test
  * @run
  */
+var InternalRunnableSuperclass = Java.type("jdk.nashorn.test.models.InternalRunnableSuperclass");
 try {
-    new (Java.type("jdk.nashorn.internal.test.models.InternalRunnable"))
+    new (InternalRunnableSuperclass.getInternalRunnableType())();
 } catch(e) {
     print(e)
 }

nashorn/test/script/trusted/JDK-8021189.js.EXPECTED

@@ -1 +1 @@
-TypeError: Can not construct jdk.nashorn.internal.test.models.InternalRunnable with the passed arguments; they do not match any of its constructor signatures.
+java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessClassInPackage.jdk.nashorn.internal.test.models")

nashorn/test/src/jdk/nashorn/test/models/InternalRunnableSuperclass.java

@@ -25,6 +25,9 @@
 
 package jdk.nashorn.test.models;
 
+import jdk.internal.dynalink.beans.StaticClass;
+import jdk.nashorn.internal.test.models.InternalRunnable;
+
 /**
  * Acts as a non-restricted superclass for a restricted class.
  *
@@ -32,4 +35,11 @@ package jdk.nashorn.test.models;
 public class InternalRunnableSuperclass {
     public final int canSeeThisField = 19;
 
+    public static Object makeInternalRunnable() {
+        return new InternalRunnable();
+    }
+
+    public static StaticClass getInternalRunnableType() {
+        return StaticClass.forClass(InternalRunnable.class);
+    }
 }