Fri, 12 Jul 2013 20:06:41 +0530
8020463: Input argument array wrapping in loadWithNewGlobal is wrong
Reviewed-by: attila, jlaskey
1.1 --- a/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java Fri Jul 12 11:58:42 2013 +0200 1.2 +++ b/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java Fri Jul 12 20:06:41 2013 +0530 1.3 @@ -52,11 +52,6 @@ 1.4 private final ScriptObject sobj; 1.5 private final ScriptObject global; 1.6 1.7 - ScriptObjectMirror(final ScriptObject sobj, final ScriptObject global) { 1.8 - this.sobj = sobj; 1.9 - this.global = global; 1.10 - } 1.11 - 1.12 @Override 1.13 public boolean equals(final Object other) { 1.14 if (other instanceof ScriptObjectMirror) { 1.15 @@ -81,25 +76,6 @@ 1.16 }); 1.17 } 1.18 1.19 - private <V> V inGlobal(final Callable<V> callable) { 1.20 - final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal(); 1.21 - final boolean globalChanged = (oldGlobal != global); 1.22 - if (globalChanged) { 1.23 - NashornScriptEngine.setNashornGlobal(global); 1.24 - } 1.25 - try { 1.26 - return callable.call(); 1.27 - } catch (final RuntimeException e) { 1.28 - throw e; 1.29 - } catch (final Exception e) { 1.30 - throw new AssertionError("Cannot happen", e); 1.31 - } finally { 1.32 - if (globalChanged) { 1.33 - NashornScriptEngine.setNashornGlobal(oldGlobal); 1.34 - } 1.35 - } 1.36 - } 1.37 - 1.38 // JSObject methods 1.39 @Override 1.40 public Object call(final String functionName, final Object... args) { 1.41 @@ -212,6 +188,8 @@ 1.42 }); 1.43 } 1.44 1.45 + // javax.script.Bindings methods 1.46 + 1.47 @Override 1.48 public void clear() { 1.49 inGlobal(new Callable<Object>() { 1.50 @@ -379,7 +357,7 @@ 1.51 public Object getProto() { 1.52 return inGlobal(new Callable<Object>() { 1.53 @Override public Object call() { 1.54 - return wrap(getScriptObject().getProto(), global); 1.55 + return wrap(sobj.getProto(), global); 1.56 } 1.57 }); 1.58 } 1.59 @@ -395,7 +373,7 @@ 1.60 public Object getOwnPropertyDescriptor(final String key) { 1.61 return inGlobal(new Callable<Object>() { 1.62 @Override public Object call() { 1.63 - return wrap(getScriptObject().getOwnPropertyDescriptor(key), global); 1.64 + return wrap(sobj.getOwnPropertyDescriptor(key), global); 1.65 } 1.66 }); 1.67 } 1.68 @@ -409,7 +387,7 @@ 1.69 public String[] getOwnKeys(final boolean all) { 1.70 return inGlobal(new Callable<String[]>() { 1.71 @Override public String[] call() { 1.72 - return getScriptObject().getOwnKeys(all); 1.73 + return sobj.getOwnKeys(all); 1.74 } 1.75 }); 1.76 } 1.77 @@ -422,7 +400,7 @@ 1.78 public ScriptObjectMirror preventExtensions() { 1.79 return inGlobal(new Callable<ScriptObjectMirror>() { 1.80 @Override public ScriptObjectMirror call() { 1.81 - getScriptObject().preventExtensions(); 1.82 + sobj.preventExtensions(); 1.83 return ScriptObjectMirror.this; 1.84 } 1.85 }); 1.86 @@ -435,7 +413,7 @@ 1.87 public boolean isExtensible() { 1.88 return inGlobal(new Callable<Boolean>() { 1.89 @Override public Boolean call() { 1.90 - return getScriptObject().isExtensible(); 1.91 + return sobj.isExtensible(); 1.92 } 1.93 }); 1.94 } 1.95 @@ -447,7 +425,7 @@ 1.96 public ScriptObjectMirror seal() { 1.97 return inGlobal(new Callable<ScriptObjectMirror>() { 1.98 @Override public ScriptObjectMirror call() { 1.99 - getScriptObject().seal(); 1.100 + sobj.seal(); 1.101 return ScriptObjectMirror.this; 1.102 } 1.103 }); 1.104 @@ -460,7 +438,7 @@ 1.105 public boolean isSealed() { 1.106 return inGlobal(new Callable<Boolean>() { 1.107 @Override public Boolean call() { 1.108 - return getScriptObject().isSealed(); 1.109 + return sobj.isSealed(); 1.110 } 1.111 }); 1.112 } 1.113 @@ -472,7 +450,7 @@ 1.114 public ScriptObjectMirror freeze() { 1.115 return inGlobal(new Callable<ScriptObjectMirror>() { 1.116 @Override public ScriptObjectMirror call() { 1.117 - getScriptObject().freeze(); 1.118 + sobj.freeze(); 1.119 return ScriptObjectMirror.this; 1.120 } 1.121 }); 1.122 @@ -485,7 +463,7 @@ 1.123 public boolean isFrozen() { 1.124 return inGlobal(new Callable<Boolean>() { 1.125 @Override public Boolean call() { 1.126 - return getScriptObject().isFrozen(); 1.127 + return sobj.isFrozen(); 1.128 } 1.129 }); 1.130 } 1.131 @@ -507,12 +485,39 @@ 1.132 1.133 return inGlobal(new Callable<Boolean>() { 1.134 @Override public Boolean call() { 1.135 - return getScriptObject().isInstance(instance.getScriptObject()); 1.136 + return sobj.isInstance(instance.sobj); 1.137 } 1.138 }); 1.139 } 1.140 1.141 /** 1.142 + * is this a function object? 1.143 + * 1.144 + * @return if this mirror wraps a ECMAScript function instance 1.145 + */ 1.146 + public boolean isFunction() { 1.147 + return sobj instanceof ScriptFunction; 1.148 + } 1.149 + 1.150 + /** 1.151 + * is this a 'use strict' function object? 1.152 + * 1.153 + * @return true if this mirror represents a ECMAScript 'use strict' function 1.154 + */ 1.155 + public boolean isStrictFunction() { 1.156 + return isFunction() && ((ScriptFunction)sobj).isStrict(); 1.157 + } 1.158 + 1.159 + /** 1.160 + * is this an array object? 1.161 + * 1.162 + * @return if this mirror wraps a ECMAScript array object 1.163 + */ 1.164 + public boolean isArray() { 1.165 + return sobj.isArray(); 1.166 + } 1.167 + 1.168 + /** 1.169 * Utility to check if given object is ECMAScript undefined value 1.170 * 1.171 * @param obj object to check 1.172 @@ -523,35 +528,6 @@ 1.173 } 1.174 1.175 /** 1.176 - * is this a function object? 1.177 - * 1.178 - * @return if this mirror wraps a ECMAScript function instance 1.179 - */ 1.180 - public boolean isFunction() { 1.181 - return getScriptObject() instanceof ScriptFunction; 1.182 - } 1.183 - 1.184 - /** 1.185 - * is this a 'use strict' function object? 1.186 - * 1.187 - * @return true if this mirror represents a ECMAScript 'use strict' function 1.188 - */ 1.189 - public boolean isStrictFunction() { 1.190 - return isFunction() && ((ScriptFunction)getScriptObject()).isStrict(); 1.191 - } 1.192 - 1.193 - /** 1.194 - * is this an array object? 1.195 - * 1.196 - * @return if this mirror wraps a ECMAScript array object 1.197 - */ 1.198 - public boolean isArray() { 1.199 - return getScriptObject().isArray(); 1.200 - } 1.201 - 1.202 - // These are public only so that Context can access these. 1.203 - 1.204 - /** 1.205 * Make a script object mirror on given object if needed. 1.206 * 1.207 * @param obj object to be wrapped 1.208 @@ -621,6 +597,12 @@ 1.209 } 1.210 1.211 // package-privates below this. 1.212 + 1.213 + ScriptObjectMirror(final ScriptObject sobj, final ScriptObject global) { 1.214 + this.sobj = sobj; 1.215 + this.global = global; 1.216 + } 1.217 + 1.218 ScriptObject getScriptObject() { 1.219 return sobj; 1.220 } 1.221 @@ -628,4 +610,25 @@ 1.222 static Object translateUndefined(Object obj) { 1.223 return (obj == ScriptRuntime.UNDEFINED)? null : obj; 1.224 } 1.225 + 1.226 + // internals only below this. 1.227 + private <V> V inGlobal(final Callable<V> callable) { 1.228 + final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal(); 1.229 + final boolean globalChanged = (oldGlobal != global); 1.230 + if (globalChanged) { 1.231 + NashornScriptEngine.setNashornGlobal(global); 1.232 + } 1.233 + try { 1.234 + return callable.call(); 1.235 + } catch (final RuntimeException e) { 1.236 + throw e; 1.237 + } catch (final Exception e) { 1.238 + throw new AssertionError("Cannot happen", e); 1.239 + } finally { 1.240 + if (globalChanged) { 1.241 + NashornScriptEngine.setNashornGlobal(oldGlobal); 1.242 + } 1.243 + } 1.244 + } 1.245 + 1.246 }
2.1 --- a/src/jdk/nashorn/internal/runtime/Context.java Fri Jul 12 11:58:42 2013 +0200 2.2 +++ b/src/jdk/nashorn/internal/runtime/Context.java Fri Jul 12 20:06:41 2013 +0530 2.3 @@ -526,11 +526,13 @@ 2.4 }); 2.5 setGlobalTrusted(newGlobal); 2.6 2.7 - final Object[] wrapped = args == null? ScriptRuntime.EMPTY_ARRAY : ScriptObjectMirror.wrapArray(args, newGlobal); 2.8 + final Object[] wrapped = args == null? ScriptRuntime.EMPTY_ARRAY : ScriptObjectMirror.wrapArray(args, oldGlobal); 2.9 newGlobal.put("arguments", ((GlobalObject)newGlobal).wrapAsObject(wrapped)); 2.10 2.11 try { 2.12 - return ScriptObjectMirror.wrap(load(newGlobal, from), newGlobal); 2.13 + // wrap objects from newGlobal's world as mirrors - but if result 2.14 + // is from oldGlobal's world, unwrap it! 2.15 + return ScriptObjectMirror.unwrap(ScriptObjectMirror.wrap(load(newGlobal, from), newGlobal), oldGlobal); 2.16 } finally { 2.17 setGlobalTrusted(oldGlobal); 2.18 }
3.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 3.2 +++ b/test/script/basic/JDK-8020463.js Fri Jul 12 20:06:41 2013 +0530 3.3 @@ -0,0 +1,54 @@ 3.4 +/* 3.5 + * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved. 3.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 3.7 + * 3.8 + * This code is free software; you can redistribute it and/or modify it 3.9 + * under the terms of the GNU General Public License version 2 only, as 3.10 + * published by the Free Software Foundation. 3.11 + * 3.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 3.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 3.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 3.15 + * version 2 for more details (a copy is included in the LICENSE file that 3.16 + * accompanied this code). 3.17 + * 3.18 + * You should have received a copy of the GNU General Public License version 3.19 + * 2 along with this work; if not, write to the Free Software Foundation, 3.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 3.21 + * 3.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 3.23 + * or visit www.oracle.com if you need additional information or have any 3.24 + * questions. 3.25 + */ 3.26 + 3.27 +/** 3.28 + * JDK-8020463: Input argument array wrapping in loadWithNewGlobal is wrong 3.29 + * 3.30 + * @test 3.31 + * @run 3.32 + */ 3.33 + 3.34 +loadWithNewGlobal({ 3.35 + name: "test", 3.36 + script: "arguments[0]();" 3.37 +}, func); 3.38 + 3.39 +function func() { 3.40 + try { 3.41 + foo; 3.42 + } catch (e) { 3.43 + if (! (e instanceof ReferenceError)) { 3.44 + fail("FAILED: expected ReferenceError!"); 3.45 + } 3.46 + } 3.47 +} 3.48 + 3.49 +// identity 3.50 +var result = loadWithNewGlobal({ 3.51 + name: "test2", 3.52 + script: "arguments[0]" 3.53 +}, this); 3.54 + 3.55 +if (result !== this) { 3.56 + fail("FAILED: expected global to be returned 'as is'"); 3.57 +}
4.1 --- a/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java Fri Jul 12 11:58:42 2013 +0200 4.2 +++ b/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java Fri Jul 12 20:06:41 2013 +0530 4.3 @@ -27,7 +27,10 @@ 4.4 4.5 import static org.testng.Assert.fail; 4.6 4.7 +import java.util.Objects; 4.8 +import javax.script.Invocable; 4.9 import javax.script.ScriptEngine; 4.10 +import javax.script.ScriptException; 4.11 import javax.script.ScriptEngineManager; 4.12 import org.testng.annotations.Test; 4.13 4.14 @@ -44,6 +47,7 @@ 4.15 public void securityPackagesTest() { 4.16 if (System.getSecurityManager() == null) { 4.17 // pass vacuously 4.18 + return; 4.19 } 4.20 4.21 final ScriptEngineManager m = new ScriptEngineManager(); 4.22 @@ -64,6 +68,7 @@ 4.23 public void securityJavaTypeTest() { 4.24 if (System.getSecurityManager() == null) { 4.25 // pass vacuously 4.26 + return; 4.27 } 4.28 4.29 final ScriptEngineManager m = new ScriptEngineManager(); 4.30 @@ -84,6 +89,7 @@ 4.31 public void securityClassForNameTest() { 4.32 if (System.getSecurityManager() == null) { 4.33 // pass vacuously 4.34 + return; 4.35 } 4.36 4.37 final ScriptEngineManager m = new ScriptEngineManager(); 4.38 @@ -104,6 +110,7 @@ 4.39 public void securitySystemExit() { 4.40 if (System.getSecurityManager() == null) { 4.41 // pass vacuously 4.42 + return; 4.43 } 4.44 4.45 final ScriptEngineManager m = new ScriptEngineManager(); 4.46 @@ -124,6 +131,7 @@ 4.47 public void securitySystemLoadLibrary() { 4.48 if (System.getSecurityManager() == null) { 4.49 // pass vacuously 4.50 + return; 4.51 } 4.52 4.53 final ScriptEngineManager m = new ScriptEngineManager(); 4.54 @@ -139,4 +147,40 @@ 4.55 } 4.56 } 4.57 } 4.58 + 4.59 + @Test 4.60 + /** 4.61 + * Check that script can't implement sensitive package interfaces. 4.62 + */ 4.63 + public void checkSensitiveInterfaceImplTest() throws ScriptException { 4.64 + if (System.getSecurityManager() == null) { 4.65 + // pass vacuously 4.66 + return; 4.67 + } 4.68 + 4.69 + final ScriptEngineManager m = new ScriptEngineManager(); 4.70 + final ScriptEngine e = m.getEngineByName("nashorn"); 4.71 + final Object[] holder = new Object[1]; 4.72 + e.put("holder", holder); 4.73 + // put an empty script object into array 4.74 + e.eval("holder[0] = {}"); 4.75 + // holder[0] is an object of some subclass of ScriptObject 4.76 + Class ScriptObjectClass = holder[0].getClass().getSuperclass(); 4.77 + Class PropertyAccessClass = ScriptObjectClass.getInterfaces()[0]; 4.78 + // implementation methods for PropertyAccess class 4.79 + e.eval("function set() {}; function get() {}; function getInt(){} " + 4.80 + "function getDouble(){}; function getLong() {}; " + 4.81 + "this.delete = function () {}; function has() {}; " + 4.82 + "function hasOwnProperty() {}"); 4.83 + 4.84 + // get implementation of a restricted package interface 4.85 + try { 4.86 + log(Objects.toString(((Invocable)e).getInterface((Class<?>)PropertyAccessClass))); 4.87 + fail("should have thrown SecurityException"); 4.88 + } catch (final Exception exp) { 4.89 + if (! (exp instanceof SecurityException)) { 4.90 + fail("SecurityException expected, got " + exp); 4.91 + } 4.92 + } 4.93 + } 4.94 }
5.1 --- a/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Fri Jul 12 11:58:42 2013 +0200 5.2 +++ b/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Fri Jul 12 20:06:41 2013 +0530 5.3 @@ -945,35 +945,4 @@ 5.4 Assert.assertEquals(itf.test1(42, "a", "b"), "i == 42, strings instanceof java.lang.String[] == true, strings == [a, b]"); 5.5 Assert.assertEquals(itf.test2(44, "c", "d", "e"), "arguments[0] == 44, arguments[1] instanceof java.lang.String[] == true, arguments[1] == [c, d, e]"); 5.6 } 5.7 - 5.8 - @Test 5.9 - /** 5.10 - * Check that script can't implement sensitive package interfaces. 5.11 - */ 5.12 - public void checkSensitiveInterfaceImplTest() throws ScriptException { 5.13 - final ScriptEngineManager m = new ScriptEngineManager(); 5.14 - final ScriptEngine e = m.getEngineByName("nashorn"); 5.15 - final Object[] holder = new Object[1]; 5.16 - e.put("holder", holder); 5.17 - // put an empty script object into array 5.18 - e.eval("holder[0] = {}"); 5.19 - // holder[0] is an object of some subclass of ScriptObject 5.20 - Class ScriptObjectClass = holder[0].getClass().getSuperclass(); 5.21 - Class PropertyAccessClass = ScriptObjectClass.getInterfaces()[0]; 5.22 - // implementation methods for PropertyAccess class 5.23 - e.eval("function set() {}; function get() {}; function getInt(){} " + 5.24 - "function getDouble(){}; function getLong() {}; " + 5.25 - "this.delete = function () {}; function has() {}; " + 5.26 - "function hasOwnProperty() {}"); 5.27 - 5.28 - // get implementation of a restricted package interface 5.29 - try { 5.30 - log(Objects.toString(((Invocable)e).getInterface((Class<?>)PropertyAccessClass))); 5.31 - fail("should have thrown SecurityException"); 5.32 - } catch (final Exception exp) { 5.33 - if (! (exp instanceof SecurityException)) { 5.34 - fail("SecurityException expected, got " + exp); 5.35 - } 5.36 - } 5.37 - } 5.38 }