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

Merge pull request #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.

Author: tcare

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]);

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

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);
-