8020276: interface checks in Invocable.getInterface implementation
authorsundar
Wed, 10 Jul 2013 19:08:04 +0530
changeset 18864 c701b823ed9e
parent 18863 f582e6cdeae5
child 18865 8844964e5fc5
8020276: interface checks in Invocable.getInterface implementation Reviewed-by: jlaskey, hannesw, attila
nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java
nashorn/src/jdk/nashorn/internal/runtime/Context.java
nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java
nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java
--- a/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Wed Jul 10 10:54:19 2013 +0200
+++ b/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Wed Jul 10 19:08:04 2013 +0530
@@ -33,6 +33,7 @@
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.security.AccessController;
@@ -184,6 +185,23 @@
     }
 
     private <T> T getInterfaceInner(final Object self, final Class<T> clazz) {
+        if (clazz == null || !clazz.isInterface()) {
+            throw new IllegalArgumentException("interface Class expected");
+        }
+
+        // perform security access check as early as possible
+        final SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            if (! Modifier.isPublic(clazz.getModifiers())) {
+                throw new SecurityException("attempt to implement non-public interfce: " + clazz);
+            }
+            final String fullName = clazz.getName();
+            final int index = fullName.lastIndexOf('.');
+            if (index != -1) {
+                sm.checkPackageAccess(fullName.substring(0, index));
+            }
+        }
+
         final ScriptObject realSelf;
         final ScriptObject ctxtGlobal = getNashornGlobalFrom(context);
         if(self == null) {
@@ -193,6 +211,7 @@
         } else {
             realSelf = (ScriptObject)self;
         }
+
         try {
             final ScriptObject oldGlobal = getNashornGlobal();
             try {
--- a/nashorn/src/jdk/nashorn/internal/runtime/Context.java	Wed Jul 10 10:54:19 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/Context.java	Wed Jul 10 19:08:04 2013 +0530
@@ -36,6 +36,7 @@
 import java.io.PrintWriter;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
+import java.util.concurrent.atomic.AtomicLong;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.AccessControlContext;
@@ -203,7 +204,7 @@
     private final ErrorManager errors;
 
     /** Unique id for script. Used only when --loader-per-compile=false */
-    private long uniqueScriptId;
+    private final AtomicLong uniqueScriptId;
 
     private static final ClassLoader myLoader = Context.class.getClassLoader();
     private static final StructureLoader sharedLoader;
@@ -263,7 +264,13 @@
         this.env       = new ScriptEnvironment(options, out, err);
         this._strict   = env._strict;
         this.appLoader = appLoader;
-        this.scriptLoader = env._loader_per_compile? null : createNewLoader();
+        if (env._loader_per_compile) {
+            this.scriptLoader = null;
+            this.uniqueScriptId = null;
+        } else {
+            this.scriptLoader = createNewLoader();
+            this.uniqueScriptId = new AtomicLong();
+        }
         this.errors    = errors;
 
         // if user passed -classpath option, make a class loader with that and set it as
@@ -825,7 +832,7 @@
         return new Global(this);
     }
 
-    private synchronized long getUniqueScriptId() {
-        return uniqueScriptId++;
+    private long getUniqueScriptId() {
+        return uniqueScriptId.getAndIncrement();
     }
 }
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java	Wed Jul 10 10:54:19 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java	Wed Jul 10 19:08:04 2013 +0530
@@ -99,6 +99,17 @@
      */
     public static StaticClass getAdapterClassFor(final Class<?>[] types, ScriptObject classOverrides) {
         assert types != null && types.length > 0;
+        final SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            for (Class type : types) {
+                // check for restricted package access
+                final String fullName = type.getName();
+                final int index = fullName.lastIndexOf('.');
+                if (index != -1) {
+                    sm.checkPackageAccess(fullName.substring(0, index));
+                }
+            }
+        }
         return getAdapterInfo(types).getAdapterClassFor(classOverrides);
     }
 
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Wed Jul 10 10:54:19 2013 +0200
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Wed Jul 10 19:08:04 2013 +0530
@@ -36,6 +36,7 @@
 import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 import javax.script.Bindings;
 import javax.script.Compilable;
@@ -345,6 +346,23 @@
     }
 
     @Test
+    /**
+     * Try passing non-interface Class object for interface implementation.
+     */
+    public void getNonInterfaceGetInterfaceTest() {
+        final ScriptEngineManager manager = new ScriptEngineManager();
+        final ScriptEngine engine = manager.getEngineByName("nashorn");
+        try {
+            log(Objects.toString(((Invocable)engine).getInterface(Object.class)));
+            fail("Should have thrown IllegalArgumentException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof IllegalArgumentException)) {
+                fail("IllegalArgumentException expected, got " + exp);
+            }
+        }
+    }
+
+    @Test
     public void accessGlobalTest() {
         final ScriptEngineManager m = new ScriptEngineManager();
         final ScriptEngine e = m.getEngineByName("nashorn");
@@ -927,4 +945,35 @@
         Assert.assertEquals(itf.test1(42, "a", "b"), "i == 42, strings instanceof java.lang.String[] == true, strings == [a, b]");
         Assert.assertEquals(itf.test2(44, "c", "d", "e"), "arguments[0] == 44, arguments[1] instanceof java.lang.String[] == true, arguments[1] == [c, d, e]");
     }
+
+    @Test
+    /**
+     * Check that script can't implement sensitive package interfaces.
+     */
+    public void checkSensitiveInterfaceImplTest() throws ScriptException {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+        final Object[] holder = new Object[1];
+        e.put("holder", holder);
+        // put an empty script object into array
+        e.eval("holder[0] = {}");
+        // holder[0] is an object of some subclass of ScriptObject
+        Class ScriptObjectClass = holder[0].getClass().getSuperclass();
+        Class PropertyAccessClass = ScriptObjectClass.getInterfaces()[0];
+        // implementation methods for PropertyAccess class
+        e.eval("function set() {}; function get() {}; function getInt(){} " +
+               "function getDouble(){}; function getLong() {}; " +
+               "this.delete = function () {}; function has() {}; " +
+               "function hasOwnProperty() {}");
+
+        // get implementation of a restricted package interface
+        try {
+            log(Objects.toString(((Invocable)e).getInterface(PropertyAccessClass)));
+            fail("should have thrown SecurityException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof SecurityException)) {
+                fail("SecurityException expected, got " + exp);
+            }
+        }
+    }
 }