From f95aa762f89adb4c5ae187e2ee11c37add784ee9 Mon Sep 17 00:00:00 2001 From: Suwei Chen Date: Tue, 9 May 2017 15:47:17 -0700 Subject: [PATCH] [CVE-2017-0252] CopyNativeIntArrayElementsToVar overflow Array length mutation introduced by copy-from-prototype reentrancy can cause heap overflow in concat fast paths. Fix by excluding copy-from-prototype cases from fast paths. Adjust unit tests to avoid excessively long run time. --- lib/Runtime/Library/JavascriptArray.cpp | 24 +++++++++------ test/Array/concat2.baseline | 40 ++++++++++++------------- test/Array/concat2.js | 2 +- test/es6/es6toLength.js | 6 ++-- 4 files changed, 39 insertions(+), 33 deletions(-) diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp index 3263a040074..3ac89c23154 100644 --- a/lib/Runtime/Library/JavascriptArray.cpp +++ b/lib/Runtime/Library/JavascriptArray.cpp @@ -3066,7 +3066,7 @@ namespace Js } if (pDestArray && JavascriptArray::IsDirectAccessArray(aItem) && JavascriptArray::IsDirectAccessArray(pDestArray) - && BigIndex(idxDest + JavascriptArray::FromVar(aItem)->length).IsSmallIndex()) // Fast path + && BigIndex(idxDest + JavascriptArray::FromVar(aItem)->length).IsSmallIndex() && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path { if (JavascriptNativeIntArray::Is(aItem)) { @@ -3200,10 +3200,11 @@ namespace Js for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++) { Var aItem = args[idxArg]; + BOOL isConcatSpreadable = false; if (scriptContext->GetConfig()->IsES6IsConcatSpreadableEnabled()) { - JS_REENTRANT(jsReentLock, BOOL isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); + JS_REENTRANT(jsReentLock, isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); if (!JavascriptNativeIntArray::Is(pDestArray)) { ConcatArgs(pDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest); @@ -3222,7 +3223,7 @@ namespace Js } } - if (JavascriptNativeIntArray::Is(aItem)) // Fast path + if (JavascriptNativeIntArray::Is(aItem) && !JavascriptNativeIntArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path { JavascriptNativeIntArray* pItemArray = JavascriptNativeIntArray::FromVar(aItem); @@ -3258,7 +3259,9 @@ namespace Js else { JavascriptArray *pVarDestArray = JavascriptNativeIntArray::ConvertToVarArray(pDestArray); - JS_REENTRANT(jsReentLock, ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest)); + BigIndex length; + JS_REENTRANT(jsReentLock, length = OP_GetLength(aItem, scriptContext), + ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest, isConcatSpreadable, length)); return pVarDestArray; } } @@ -3276,10 +3279,11 @@ namespace Js for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++) { Var aItem = args[idxArg]; + BOOL isConcatSpreadable = false; if (scriptContext->GetConfig()->IsES6IsConcatSpreadableEnabled()) { - JS_REENTRANT(jsReentLock, BOOL isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); + JS_REENTRANT(jsReentLock, isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); if (!JavascriptNativeFloatArray::Is(pDestArray)) { ConcatArgs(pDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest); @@ -3299,10 +3303,10 @@ namespace Js } } - bool converted; + bool converted = false; if (JavascriptArray::IsAnyArray(aItem) || remoteTypeIds[idxArg] == TypeIds_Array) { - if (JavascriptNativeIntArray::Is(aItem)) // Fast path + if (JavascriptNativeIntArray::Is(aItem) && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path { JavascriptNativeIntArray *pIntArray = JavascriptNativeIntArray::FromVar(aItem); @@ -3310,7 +3314,7 @@ namespace Js idxDest = idxDest + pIntArray->length; } - else if (JavascriptNativeFloatArray::Is(aItem)) + else if (JavascriptNativeFloatArray::Is(aItem) && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) { JavascriptNativeFloatArray* pItemArray = JavascriptNativeFloatArray::FromVar(aItem); @@ -3322,7 +3326,9 @@ namespace Js { JavascriptArray *pVarDestArray = JavascriptNativeFloatArray::ConvertToVarArray(pDestArray); - JS_REENTRANT(jsReentLock, ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest)); + BigIndex length; + JS_REENTRANT(jsReentLock, length = OP_GetLength(aItem, scriptContext), + ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest, isConcatSpreadable, length)); return pVarDestArray; } diff --git a/test/Array/concat2.baseline b/test/Array/concat2.baseline index b7afadd0654..23da298ab0f 100644 --- a/test/Array/concat2.baseline +++ b/test/Array/concat2.baseline @@ -1,26 +1,26 @@ -------concat Small------------- - concat 101, 102, 103, 104, 105 -length: 2147483647 - 2147483641: 100 - 2147483642: 101 - 2147483643: 102 - 2147483644: 103 - 2147483645: 104 - 2147483646: 105 +length: 505 + 499: 100 + 500: 101 + 501: 102 + 502: 103 + 503: 104 + 504: 105 - arr.concat(arr) -length: 4294967294 - 2147483641: 100 - 2147483642: 101 - 2147483643: 102 - 2147483644: 103 - 2147483645: 104 - 2147483646: 105 - 4294967288: 100 - 4294967289: 101 - 4294967290: 102 - 4294967291: 103 - 4294967292: 104 - 4294967293: 105 +length: 1010 + 499: 100 + 500: 101 + 501: 102 + 502: 103 + 503: 104 + 504: 105 + 1004: 100 + 1005: 101 + 1006: 102 + 1007: 103 + 1008: 104 + 1009: 105 -------test prototype lookup------------- a: 200,101,202,203,204,105,106,207 r: 200,101,202,203,204,105,106,207,300,301,302,303,304 diff --git a/test/Array/concat2.js b/test/Array/concat2.js index 590f09ae14b..ae7ae3ed72a 100644 --- a/test/Array/concat2.js +++ b/test/Array/concat2.js @@ -38,7 +38,7 @@ function test_concat(size) { } echo("-------concat Small-------------"); -test_concat(2147483642); +test_concat(500); // Fix for MSRC 33319 changes concat to skip a fast path if the index we're copying into is a BigIndex. // Disable the large portion of this test since this change makes such a test run for hours. diff --git a/test/es6/es6toLength.js b/test/es6/es6toLength.js index c3ba3c8c5d3..d32bde0dcd4 100644 --- a/test/es6/es6toLength.js +++ b/test/es6/es6toLength.js @@ -12,12 +12,12 @@ var tests = [ { var c = []; c[0] = 1; - c[4294967293] = 2; + c[100] = 2; var oNeg = { length : -1, 0 : 3, 1: 4, [Symbol.isConcatSpreadable] : true}; c = c.concat(oNeg); assert.areEqual(1, c[0], "confirm indices of array concated to did not change") - assert.areEqual(2, c[4294967293], "confirm indices of array concated to did not change"); - assert.areEqual(undefined, c[4294967294], "Length of oNeg is coerced to 0 nothing is concated here"); + assert.areEqual(2, c[100], "confirm indices of array concated to did not change"); + assert.areEqual(undefined, c[101], "Length of oNeg is coerced to 0 nothing is concated here"); } }, {