From eecf271764ce0ee8ea58c2ec9c22bc2dd69861e7 Mon Sep 17 00:00:00 2001 From: Suwei Chen Date: Tue, 27 Sep 2016 10:19:07 -0700 Subject: [PATCH] Heap overflow in Array.prototype.reverse In Array.prototype.reverse, array length is cached and used in ReverseHelper(). ReverseHelper() could invoke FillFromPrototypes(), which can cause a side-effect on the array, including changing its length. Therefore, the use of cached array length to calculate segment left index could result in overflow. Fix by clamping array length at zero. --- lib/Runtime/Library/JavascriptArray.cpp | 2 +- test/Array/Array_TypeConfusion_bugs.js | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp index 4ec60bef4d5..96cab2da0c5 100644 --- a/lib/Runtime/Library/JavascriptArray.cpp +++ b/lib/Runtime/Library/JavascriptArray.cpp @@ -5202,7 +5202,7 @@ namespace Js ((SparseArraySegment*)seg)->ReverseSegment(recycler); } - seg->left = ((uint32)length) - (seg->left + seg->length); + seg->left = ((uint32)length) > (seg->left + seg->length) ? ((uint32)length) - (seg->left + seg->length) : 0; seg->next = prevSeg; // Make sure size doesn't overlap with next segment. diff --git a/test/Array/Array_TypeConfusion_bugs.js b/test/Array/Array_TypeConfusion_bugs.js index d432607f1aa..e1867424929 100644 --- a/test/Array/Array_TypeConfusion_bugs.js +++ b/test/Array/Array_TypeConfusion_bugs.js @@ -574,5 +574,24 @@ var tests = [ assert.areEqual([0x41424344], Array.prototype.slice.call(y)); } }, + { + name: "[MSRC34994,35226] heap overflow in Array.prototype.reverse", + body: function () + { + var count = 0; + arr = new Array(100); + var desc = Object.getOwnPropertyDescriptor(Array.prototype, 1); + Object.defineProperty(Array.prototype, 1, { get: function () { + count++; + if (count == 1) { + arr.push(null); + } + }}); + + arr.reverse(); + restorePropertyFromDescriptor(Array.prototype, 1, desc); + assert.areEqual(101, arr.length); + } + }, ]; testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });