Skip to content

Commit f95aa76

Browse files
Suwei Chenrajatd
Suwei Chen
authored and
rajatd
committed
[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.
1 parent 9587507 commit f95aa76

4 files changed

Lines changed: 39 additions & 33 deletions

File tree

‎lib/Runtime/Library/JavascriptArray.cpp‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3066,7 +3066,7 @@ namespace Js
30663066
}
30673067

30683068
if (pDestArray && JavascriptArray::IsDirectAccessArray(aItem) && JavascriptArray::IsDirectAccessArray(pDestArray)
3069-
&& BigIndex(idxDest + JavascriptArray::FromVar(aItem)->length).IsSmallIndex()) // Fast path
3069+
&& BigIndex(idxDest + JavascriptArray::FromVar(aItem)->length).IsSmallIndex() && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path
30703070
{
30713071
if (JavascriptNativeIntArray::Is(aItem))
30723072
{
@@ -3200,10 +3200,11 @@ namespace Js
32003200
for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++)
32013201
{
32023202
Var aItem = args[idxArg];
3203+
BOOL isConcatSpreadable = false;
32033204

32043205
if (scriptContext->GetConfig()->IsES6IsConcatSpreadableEnabled())
32053206
{
3206-
JS_REENTRANT(jsReentLock, BOOL isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem));
3207+
JS_REENTRANT(jsReentLock, isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem));
32073208
if (!JavascriptNativeIntArray::Is(pDestArray))
32083209
{
32093210
ConcatArgs<uint>(pDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest);
@@ -3222,7 +3223,7 @@ namespace Js
32223223
}
32233224
}
32243225

3225-
if (JavascriptNativeIntArray::Is(aItem)) // Fast path
3226+
if (JavascriptNativeIntArray::Is(aItem) && !JavascriptNativeIntArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path
32263227
{
32273228
JavascriptNativeIntArray* pItemArray = JavascriptNativeIntArray::FromVar(aItem);
32283229

@@ -3258,7 +3259,9 @@ namespace Js
32583259
else
32593260
{
32603261
JavascriptArray *pVarDestArray = JavascriptNativeIntArray::ConvertToVarArray(pDestArray);
3261-
JS_REENTRANT(jsReentLock, ConcatArgs<uint>(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest));
3262+
BigIndex length;
3263+
JS_REENTRANT(jsReentLock, length = OP_GetLength(aItem, scriptContext),
3264+
ConcatArgs<uint>(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest, isConcatSpreadable, length));
32623265
return pVarDestArray;
32633266
}
32643267
}
@@ -3276,10 +3279,11 @@ namespace Js
32763279
for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++)
32773280
{
32783281
Var aItem = args[idxArg];
3282+
BOOL isConcatSpreadable = false;
32793283

32803284
if (scriptContext->GetConfig()->IsES6IsConcatSpreadableEnabled())
32813285
{
3282-
JS_REENTRANT(jsReentLock, BOOL isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem));
3286+
JS_REENTRANT(jsReentLock, isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem));
32833287
if (!JavascriptNativeFloatArray::Is(pDestArray))
32843288
{
32853289
ConcatArgs<uint>(pDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest);
@@ -3299,18 +3303,18 @@ namespace Js
32993303
}
33003304
}
33013305

3302-
bool converted;
3306+
bool converted = false;
33033307
if (JavascriptArray::IsAnyArray(aItem) || remoteTypeIds[idxArg] == TypeIds_Array)
33043308
{
3305-
if (JavascriptNativeIntArray::Is(aItem)) // Fast path
3309+
if (JavascriptNativeIntArray::Is(aItem) && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path
33063310
{
33073311
JavascriptNativeIntArray *pIntArray = JavascriptNativeIntArray::FromVar(aItem);
33083312

33093313
JS_REENTRANT(jsReentLock, converted = CopyNativeIntArrayElementsToFloat(pDestArray, idxDest, pIntArray));
33103314

33113315
idxDest = idxDest + pIntArray->length;
33123316
}
3313-
else if (JavascriptNativeFloatArray::Is(aItem))
3317+
else if (JavascriptNativeFloatArray::Is(aItem) && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes())
33143318
{
33153319
JavascriptNativeFloatArray* pItemArray = JavascriptNativeFloatArray::FromVar(aItem);
33163320

@@ -3322,7 +3326,9 @@ namespace Js
33223326
{
33233327
JavascriptArray *pVarDestArray = JavascriptNativeFloatArray::ConvertToVarArray(pDestArray);
33243328

3325-
JS_REENTRANT(jsReentLock, ConcatArgs<uint>(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest));
3329+
BigIndex length;
3330+
JS_REENTRANT(jsReentLock, length = OP_GetLength(aItem, scriptContext),
3331+
ConcatArgs<uint>(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest, isConcatSpreadable, length));
33263332

33273333
return pVarDestArray;
33283334
}

‎test/Array/concat2.baseline‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,26 @@
11
-------concat Small-------------
22
- concat 101, 102, 103, 104, 105
3-
length: 2147483647
4-
2147483641: 100
5-
2147483642: 101
6-
2147483643: 102
7-
2147483644: 103
8-
2147483645: 104
9-
2147483646: 105
3+
length: 505
4+
499: 100
5+
500: 101
6+
501: 102
7+
502: 103
8+
503: 104
9+
504: 105
1010
- arr.concat(arr)
11-
length: 4294967294
12-
2147483641: 100
13-
2147483642: 101
14-
2147483643: 102
15-
2147483644: 103
16-
2147483645: 104
17-
2147483646: 105
18-
4294967288: 100
19-
4294967289: 101
20-
4294967290: 102
21-
4294967291: 103
22-
4294967292: 104
23-
4294967293: 105
11+
length: 1010
12+
499: 100
13+
500: 101
14+
501: 102
15+
502: 103
16+
503: 104
17+
504: 105
18+
1004: 100
19+
1005: 101
20+
1006: 102
21+
1007: 103
22+
1008: 104
23+
1009: 105
2424
-------test prototype lookup-------------
2525
a: 200,101,202,203,204,105,106,207
2626
r: 200,101,202,203,204,105,106,207,300,301,302,303,304

‎test/Array/concat2.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ function test_concat(size) {
3838
}
3939

4040
echo("-------concat Small-------------");
41-
test_concat(2147483642);
41+
test_concat(500);
4242

4343
// Fix for MSRC 33319 changes concat to skip a fast path if the index we're copying into is a BigIndex.
4444
// Disable the large portion of this test since this change makes such a test run for hours.

‎test/es6/es6toLength.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ var tests = [
1212
{
1313
var c = [];
1414
c[0] = 1;
15-
c[4294967293] = 2;
15+
c[100] = 2;
1616
var oNeg = { length : -1, 0 : 3, 1: 4, [Symbol.isConcatSpreadable] : true};
1717
c = c.concat(oNeg);
1818
assert.areEqual(1, c[0], "confirm indices of array concated to did not change")
19-
assert.areEqual(2, c[4294967293], "confirm indices of array concated to did not change");
20-
assert.areEqual(undefined, c[4294967294], "Length of oNeg is coerced to 0 nothing is concated here");
19+
assert.areEqual(2, c[100], "confirm indices of array concated to did not change");
20+
assert.areEqual(undefined, c[101], "Length of oNeg is coerced to 0 nothing is concated here");
2121
}
2222
},
2323
{

0 commit comments

Comments
 (0)