[MERGE chakra-core#2201 @tcare] Array.prototype.unshift does not marshal parameters correctly

tcare · tcare · commit 96c8b123204f · 2017-01-06T16:46:16.000-08:00
Merge pull request chakra-core#2201 from tcare:unshift Fixes OS 9357224. Array.prototype.unshift prepends array values to 'this' array by calling JavascriptArray::FillFromArgs. FillFromArgs makes the assumption that values are already marshalled to the same context as 'this'. It is possible to have a situation in Array.prototype.unshift where the source array is in another context than the parameters passed in, triggering the assert. Other users of FillFromArgs (e.g. new Array()) don't seem to be able to hit this situation. Fix is to marshal values as we iterate in FillFromArgs.
diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp
@@ -7563,7 +7563,7 @@ namespace Js
         {
            return res;
         }
-        if (JavascriptArray::Is(args[0]))
+        if (JavascriptArray::Is(args[0]) && !JavascriptArray::FromVar(args[0])->IsCrossSiteObject())
         {
 #if ENABLE_COPYONACCESS_ARRAY
             JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(args[0]);
diff --git a/test/Array/shift_unshift.baseline b/test/Array/shift_unshift.baseline
@@ -48,6 +48,7 @@ e instanceOf TypeError = true
 a.length = 1
 ary.length = 18
 arr.length = 6
+Crosssite new length: 2
 Overridden unshift
 Overridden unshift
 Overridden unshift
diff --git a/test/Array/shift_unshift.js b/test/Array/shift_unshift.js
@@ -140,6 +140,14 @@ function test1(arr)
 
 WScript.Echo("arr.length = " + test1(new Array(10)));
 
+// OS 9357224: Array.prototype.unshift does not marshal parameters correctly
+function crossSiteUnshift() {
+    var sc0 = WScript.LoadScript('', 'samethread');
+    sc0.ary = [1];
+    return sc0.eval('Array.prototype.unshift.call(ary, null)');
+}
+WScript.Echo("Crosssite new length: " + crossSiteUnshift()); // 2
+
 //
 // To check bailouts for inlined unshift
 //
@@ -164,4 +172,3 @@ function foo()
 Array.prototype.unshift = function(){WScript.Echo ("Overridden unshift")};
 foo();
 WScript.Echo (a);
-

Original file line number	Diff line number	Diff line change
`@@ -7563,7 +7563,7 @@ namespace Js`
`7563`	`7563`	`{`
`7564`	`7564`	`return res;`
`7565`	`7565`	`}`
`7566`		`- if (JavascriptArray::Is(args[0]))`
	`7566`	`+ if (JavascriptArray::Is(args[0]) && !JavascriptArray::FromVar(args[0])->IsCrossSiteObject())`
`7567`	`7567`	`{`
`7568`	`7568`	`#if ENABLE_COPYONACCESS_ARRAY`
`7569`	`7569`	`JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(args[0]);`