V8: OOB write in Array.prototype.map builtin |
||||||||||||||||
Issue description
The attached PoC causes OOB writes from a call to Array.prototype.map.
In builtins/builtins-array-gen.cc,
void GenerateIteratingArrayBuiltinBody(
const char* name, const BuiltinResultGenerator& generator,
const CallResultProcessor& processor, const PostLoopAction& action,
const Callable& slow_case_continuation,
ForEachDirection direction = ForEachDirection::kForward) {
…
merged_length.Bind(LoadJSArrayLength(o()));
Goto(&has_length);
...
BIND(&has_length);
len_ = merged_length.value();
The length of the receiver JSArray (|o_|) is saved in |len_|.
Later on,
a_.Bind(generator(this));
Which creates and saves the result array of size |len_| to |a_|:
Node* MapResultGenerator() {
// 5. Let A be ? ArraySpeciesCreate(O, len).
return ArraySpeciesCreate(context(), o(), len_);
}
If the receiver is an array of a type extended from Array, we can override the species attribute to return another Array-extended type that allocates less than the requested |len_|.
Arrays created from these 2 types are still considered fast JSArrays, and later on as the map results are being written, we get OOB writes because the result JSArray is too small:
Node* MapProcessor(Node* k_value, Node* k) {
....
BIND(&fast);
{
kind = EnsureArrayPushable(a(), &runtime);
elements = LoadElements(a());
GotoIf(IsElementsKindGreaterThan(kind, FAST_HOLEY_SMI_ELEMENTS),
&object_push_pre);
TryStoreArrayElement(FAST_SMI_ELEMENTS, mode, &runtime, elements, k,
mappedValue);
Goto(&finished);
}
,
Apr 27 2017
,
Apr 27 2017
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=4633630698897408
,
Apr 27 2017
I reverted this on 5.9 for now: https://chromium.googlesource.com/v8/v8/+/521a770035214bce64b6c7990f14e11a20b07c2b
,
Apr 27 2017
,
Apr 27 2017
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5719933557407744
,
Apr 27 2017
(the uploaded testcase for the linux_asan_chrome_v8 in #3 is a .js file. I put it in a html and reuploaded it).
,
Apr 27 2017
,
Apr 27 2017
,
Apr 27 2017
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=6708869104664576
,
Apr 27 2017
,
Apr 27 2017
Thanks for the bug Oliver - clearly, we need to recognize this case. My first thought is to exclude any arrays that override the species constructor. We'll address it in the morning, all the best.
,
Apr 27 2017
Detailed report: https://clusterfuzz.com/testcase?key=6708869104664576 Job Type: linux_asan_d8_dbg Crash Type: UNKNOWN Crash Address: 0x7faa01880000 Crash State: NULL Sanitizer: address (ASAN) Recommended Security Severity: Medium Regressed: V8: 44792:44793 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6708869104664576 See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. The recommended severity is different from what was assigned to the bug. Please double check the accuracy of the assigned severity.
,
Apr 28 2017
,
Apr 28 2017
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 28 2017
CL in flight... https://codereview.chromium.org/2846963003/
,
Apr 29 2017
I wrote an exploit for Linux (tested that it works in d8 0ca84d06f5bea390a37774da1cff7a4f5179fa22 and Chrome dev 60.0.3080.5 with --no-sandbox). Should pop /usr/bin/xcalc.
,
May 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/192984ea88badc0c02e22e528b1243a9efa46f90 commit 192984ea88badc0c02e22e528b1243a9efa46f90 Author: mvstanton <mvstanton@chromium.org> Date: Wed May 03 14:11:44 2017 Array.prototype.map write error. More care must be taken to remain on the fast path in the face of @@species constructors. BUG= chromium:716044 Review-Url: https://codereview.chromium.org/2846963003 Cr-Commit-Position: refs/heads/master@{#45065} [modify] https://crrev.com/192984ea88badc0c02e22e528b1243a9efa46f90/src/builtins/builtins-array-gen.cc [modify] https://crrev.com/192984ea88badc0c02e22e528b1243a9efa46f90/src/code-stub-assembler.h [modify] https://crrev.com/192984ea88badc0c02e22e528b1243a9efa46f90/test/mjsunit/mjsunit.status [add] https://crrev.com/192984ea88badc0c02e22e528b1243a9efa46f90/test/mjsunit/regress/regress-716044.js
,
May 3 2017
,
May 4 2017
ClusterFuzz has detected this issue as fixed in range 45064:45065. Detailed report: https://clusterfuzz.com/testcase?key=6708869104664576 Job Type: linux_asan_d8_dbg Crash Type: UNKNOWN Crash Address: 0x7faa01880000 Crash State: NULL Sanitizer: address (ASAN) Recommended Security Severity: Medium Regressed: V8: 44792:44793 Fixed: V8: 45064:45065 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6708869104664576 See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
May 4 2017
,
May 18 2017
,
May 24 2017
(removing restrict view since this was a trunk regression and has been fixed).
,
May 24 2017
,
Jun 20 2018
,
Jun 26 2018
,
Jun 26 2018
|
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by och...@chromium.org
, Apr 27 2017Status: Assigned (was: Unconfirmed)