8020463: Input argument array wrapping in loadWithNewGlobal is wrong
Reviewed-by: attila, jlaskey
--- a/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java Fri Jul 12 11:58:42 2013 +0200
+++ b/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java Fri Jul 12 20:06:41 2013 +0530
@@ -52,11 +52,6 @@
private final ScriptObject sobj;
private final ScriptObject global;
- ScriptObjectMirror(final ScriptObject sobj, final ScriptObject global) {
- this.sobj = sobj;
- this.global = global;
- }
-
@Override
public boolean equals(final Object other) {
if (other instanceof ScriptObjectMirror) {
@@ -81,25 +76,6 @@
});
}
- private <V> V inGlobal(final Callable<V> callable) {
- final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal();
- final boolean globalChanged = (oldGlobal != global);
- if (globalChanged) {
- NashornScriptEngine.setNashornGlobal(global);
- }
- try {
- return callable.call();
- } catch (final RuntimeException e) {
- throw e;
- } catch (final Exception e) {
- throw new AssertionError("Cannot happen", e);
- } finally {
- if (globalChanged) {
- NashornScriptEngine.setNashornGlobal(oldGlobal);
- }
- }
- }
-
// JSObject methods
@Override
public Object call(final String functionName, final Object... args) {
@@ -212,6 +188,8 @@
});
}
+ // javax.script.Bindings methods
+
@Override
public void clear() {
inGlobal(new Callable<Object>() {
@@ -379,7 +357,7 @@
public Object getProto() {
return inGlobal(new Callable<Object>() {
@Override public Object call() {
- return wrap(getScriptObject().getProto(), global);
+ return wrap(sobj.getProto(), global);
}
});
}
@@ -395,7 +373,7 @@
public Object getOwnPropertyDescriptor(final String key) {
return inGlobal(new Callable<Object>() {
@Override public Object call() {
- return wrap(getScriptObject().getOwnPropertyDescriptor(key), global);
+ return wrap(sobj.getOwnPropertyDescriptor(key), global);
}
});
}
@@ -409,7 +387,7 @@
public String[] getOwnKeys(final boolean all) {
return inGlobal(new Callable<String[]>() {
@Override public String[] call() {
- return getScriptObject().getOwnKeys(all);
+ return sobj.getOwnKeys(all);
}
});
}
@@ -422,7 +400,7 @@
public ScriptObjectMirror preventExtensions() {
return inGlobal(new Callable<ScriptObjectMirror>() {
@Override public ScriptObjectMirror call() {
- getScriptObject().preventExtensions();
+ sobj.preventExtensions();
return ScriptObjectMirror.this;
}
});
@@ -435,7 +413,7 @@
public boolean isExtensible() {
return inGlobal(new Callable<Boolean>() {
@Override public Boolean call() {
- return getScriptObject().isExtensible();
+ return sobj.isExtensible();
}
});
}
@@ -447,7 +425,7 @@
public ScriptObjectMirror seal() {
return inGlobal(new Callable<ScriptObjectMirror>() {
@Override public ScriptObjectMirror call() {
- getScriptObject().seal();
+ sobj.seal();
return ScriptObjectMirror.this;
}
});
@@ -460,7 +438,7 @@
public boolean isSealed() {
return inGlobal(new Callable<Boolean>() {
@Override public Boolean call() {
- return getScriptObject().isSealed();
+ return sobj.isSealed();
}
});
}
@@ -472,7 +450,7 @@
public ScriptObjectMirror freeze() {
return inGlobal(new Callable<ScriptObjectMirror>() {
@Override public ScriptObjectMirror call() {
- getScriptObject().freeze();
+ sobj.freeze();
return ScriptObjectMirror.this;
}
});
@@ -485,7 +463,7 @@
public boolean isFrozen() {
return inGlobal(new Callable<Boolean>() {
@Override public Boolean call() {
- return getScriptObject().isFrozen();
+ return sobj.isFrozen();
}
});
}
@@ -507,12 +485,39 @@
return inGlobal(new Callable<Boolean>() {
@Override public Boolean call() {
- return getScriptObject().isInstance(instance.getScriptObject());
+ return sobj.isInstance(instance.sobj);
}
});
}
/**
+ * is this a function object?
+ *
+ * @return if this mirror wraps a ECMAScript function instance
+ */
+ public boolean isFunction() {
+ return sobj instanceof ScriptFunction;
+ }
+
+ /**
+ * is this a 'use strict' function object?
+ *
+ * @return true if this mirror represents a ECMAScript 'use strict' function
+ */
+ public boolean isStrictFunction() {
+ return isFunction() && ((ScriptFunction)sobj).isStrict();
+ }
+
+ /**
+ * is this an array object?
+ *
+ * @return if this mirror wraps a ECMAScript array object
+ */
+ public boolean isArray() {
+ return sobj.isArray();
+ }
+
+ /**
* Utility to check if given object is ECMAScript undefined value
*
* @param obj object to check
@@ -523,35 +528,6 @@
}
/**
- * is this a function object?
- *
- * @return if this mirror wraps a ECMAScript function instance
- */
- public boolean isFunction() {
- return getScriptObject() instanceof ScriptFunction;
- }
-
- /**
- * is this a 'use strict' function object?
- *
- * @return true if this mirror represents a ECMAScript 'use strict' function
- */
- public boolean isStrictFunction() {
- return isFunction() && ((ScriptFunction)getScriptObject()).isStrict();
- }
-
- /**
- * is this an array object?
- *
- * @return if this mirror wraps a ECMAScript array object
- */
- public boolean isArray() {
- return getScriptObject().isArray();
- }
-
- // These are public only so that Context can access these.
-
- /**
* Make a script object mirror on given object if needed.
*
* @param obj object to be wrapped
@@ -621,6 +597,12 @@
}
// package-privates below this.
+
+ ScriptObjectMirror(final ScriptObject sobj, final ScriptObject global) {
+ this.sobj = sobj;
+ this.global = global;
+ }
+
ScriptObject getScriptObject() {
return sobj;
}
@@ -628,4 +610,25 @@
static Object translateUndefined(Object obj) {
return (obj == ScriptRuntime.UNDEFINED)? null : obj;
}
+
+ // internals only below this.
+ private <V> V inGlobal(final Callable<V> callable) {
+ final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal();
+ final boolean globalChanged = (oldGlobal != global);
+ if (globalChanged) {
+ NashornScriptEngine.setNashornGlobal(global);
+ }
+ try {
+ return callable.call();
+ } catch (final RuntimeException e) {
+ throw e;
+ } catch (final Exception e) {
+ throw new AssertionError("Cannot happen", e);
+ } finally {
+ if (globalChanged) {
+ NashornScriptEngine.setNashornGlobal(oldGlobal);
+ }
+ }
+ }
+
}
--- a/nashorn/src/jdk/nashorn/internal/runtime/Context.java Fri Jul 12 11:58:42 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/Context.java Fri Jul 12 20:06:41 2013 +0530
@@ -526,11 +526,13 @@
});
setGlobalTrusted(newGlobal);
- final Object[] wrapped = args == null? ScriptRuntime.EMPTY_ARRAY : ScriptObjectMirror.wrapArray(args, newGlobal);
+ final Object[] wrapped = args == null? ScriptRuntime.EMPTY_ARRAY : ScriptObjectMirror.wrapArray(args, oldGlobal);
newGlobal.put("arguments", ((GlobalObject)newGlobal).wrapAsObject(wrapped));
try {
- return ScriptObjectMirror.wrap(load(newGlobal, from), newGlobal);
+ // wrap objects from newGlobal's world as mirrors - but if result
+ // is from oldGlobal's world, unwrap it!
+ return ScriptObjectMirror.unwrap(ScriptObjectMirror.wrap(load(newGlobal, from), newGlobal), oldGlobal);
} finally {
setGlobalTrusted(oldGlobal);
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8020463.js Fri Jul 12 20:06:41 2013 +0530
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8020463: Input argument array wrapping in loadWithNewGlobal is wrong
+ *
+ * @test
+ * @run
+ */
+
+loadWithNewGlobal({
+ name: "test",
+ script: "arguments[0]();"
+}, func);
+
+function func() {
+ try {
+ foo;
+ } catch (e) {
+ if (! (e instanceof ReferenceError)) {
+ fail("FAILED: expected ReferenceError!");
+ }
+ }
+}
+
+// identity
+var result = loadWithNewGlobal({
+ name: "test2",
+ script: "arguments[0]"
+}, this);
+
+if (result !== this) {
+ fail("FAILED: expected global to be returned 'as is'");
+}
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java Fri Jul 12 11:58:42 2013 +0200
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java Fri Jul 12 20:06:41 2013 +0530
@@ -27,7 +27,10 @@
import static org.testng.Assert.fail;
+import java.util.Objects;
+import javax.script.Invocable;
import javax.script.ScriptEngine;
+import javax.script.ScriptException;
import javax.script.ScriptEngineManager;
import org.testng.annotations.Test;
@@ -44,6 +47,7 @@
public void securityPackagesTest() {
if (System.getSecurityManager() == null) {
// pass vacuously
+ return;
}
final ScriptEngineManager m = new ScriptEngineManager();
@@ -64,6 +68,7 @@
public void securityJavaTypeTest() {
if (System.getSecurityManager() == null) {
// pass vacuously
+ return;
}
final ScriptEngineManager m = new ScriptEngineManager();
@@ -84,6 +89,7 @@
public void securityClassForNameTest() {
if (System.getSecurityManager() == null) {
// pass vacuously
+ return;
}
final ScriptEngineManager m = new ScriptEngineManager();
@@ -104,6 +110,7 @@
public void securitySystemExit() {
if (System.getSecurityManager() == null) {
// pass vacuously
+ return;
}
final ScriptEngineManager m = new ScriptEngineManager();
@@ -124,6 +131,7 @@
public void securitySystemLoadLibrary() {
if (System.getSecurityManager() == null) {
// pass vacuously
+ return;
}
final ScriptEngineManager m = new ScriptEngineManager();
@@ -139,4 +147,40 @@
}
}
}
+
+ @Test
+ /**
+ * Check that script can't implement sensitive package interfaces.
+ */
+ public void checkSensitiveInterfaceImplTest() throws ScriptException {
+ if (System.getSecurityManager() == null) {
+ // pass vacuously
+ return;
+ }
+
+ 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((Class<?>)PropertyAccessClass)));
+ 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 Fri Jul 12 11:58:42 2013 +0200
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Fri Jul 12 20:06:41 2013 +0530
@@ -945,35 +945,4 @@
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((Class<?>)PropertyAccessClass)));
- fail("should have thrown SecurityException");
- } catch (final Exception exp) {
- if (! (exp instanceof SecurityException)) {
- fail("SecurityException expected, got " + exp);
- }
- }
- }
}