8020276: interface checks in Invocable.getInterface implementation
Reviewed-by: jlaskey, hannesw, attila
--- 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);
+ }
+ }
+ }
}