New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 716044: V8: OOB write in Array.prototype.map builtin

Reported by och...@chromium.org, Apr 27 2017 Project Member

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);
    }
 
poc.js
269 bytes View Download

Comment 1 by och...@chromium.org, Apr 27 2017

Owner: mvstan...@chromium.org
Status: Assigned (was: Unconfirmed)
It looks like this code was very recently added/enabled by default in https://chromium.googlesource.com/v8/v8.git/+/1eb0ef316103caf526f9ab80290b5ba313e232af

mvstanton@, could you please take a look? Thanks!

Comment 2 by hablich@chromium.org, Apr 27 2017

Cc: bmeu...@chromium.org
Labels: Pri-1

Comment 3 by ClusterFuzz, Apr 27 2017

Project Member
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=4633630698897408

Comment 5 by hablich@chromium.org, Apr 27 2017

Cc: hablich@chromium.org

Comment 6 by ClusterFuzz, Apr 27 2017

Project Member
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5719933557407744

Comment 7 by och...@chromium.org, 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).

Comment 8 by bmeu...@chromium.org, Apr 27 2017

Cc: danno@chromium.org

Comment 9 by palmer@chromium.org, Apr 27 2017

Labels: Security_Severity-High Security_Impact-Head OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows

Comment 10 by ClusterFuzz, Apr 27 2017

Project Member
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=6708869104664576

Comment 11 by mvstan...@chromium.org, Apr 27 2017

Status: Started (was: Assigned)

Comment 12 by mvstan...@chromium.org, 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.

Comment 13 by ClusterFuzz, Apr 27 2017

Project Member
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.

Comment 14 by sheriffbot@chromium.org, Apr 28 2017

Project Member
Labels: M-60

Comment 15 by sheriffbot@chromium.org, Apr 28 2017

Project Member
Labels: ReleaseBlock-Beta
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

Comment 17 by och...@chromium.org, 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.
exploit.html
4.1 KB View Download

Comment 19 by infe...@chromium.org, May 3 2017

Status: Fixed (was: Started)

Comment 20 by ClusterFuzz, May 4 2017

Project Member
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.

Comment 21 by sheriffbot@chromium.org, May 4 2017

Project Member
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 22 by awhalley@chromium.org, May 18 2017

Labels: -ReleaseBlock-Beta

Comment 23 by och...@chromium.org, May 24 2017

Labels: -Restrict-View-SecurityNotify
(removing restrict view since this was a trunk regression and has been fixed).

Comment 24 by och...@chromium.org, May 24 2017

Labels: allpublic

Comment 25 by danno@chromium.org, Jun 20 2018

Labels: Hotlist-Torque

Comment 26 by hablich@chromium.org, Jun 26 2018

Cc: tebbi@chromium.org

Comment 27 by hablich@chromium.org, Jun 26 2018

Cc: jarin@chromium.org

Sign in to add a comment