Project: chromium Issues People Development process History Sign in
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
Status: Fixed
Owner:
Last visit 20 days ago
Closed: May 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux, Android, Windows, Chrome, Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment
V8: OOB write in Array.prototype.map builtin
Project Member Reported by och...@chromium.org, Apr 27 Back to list
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
Owner: mvstan...@chromium.org
Status: Assigned
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!
Cc: bmeu...@chromium.org
Labels: Pri-1
Project Member Comment 3 by clusterf...@chromium.org, Apr 27
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=4633630698897408
Cc: hablich@chromium.org
Project Member Comment 6 by clusterf...@chromium.org, Apr 27
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5719933557407744
(the uploaded testcase for the linux_asan_chrome_v8 in #3 is a .js file. I put it in a html and reuploaded it).
Cc: danno@chromium.org
Labels: Security_Severity-High Security_Impact-Head OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Project Member Comment 10 by clusterf...@chromium.org, Apr 27
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=6708869104664576
Status: Started
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.
Project Member Comment 13 by clusterf...@chromium.org, Apr 27
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.

Project Member Comment 14 by sheriffbot@chromium.org, Apr 28
Labels: M-60
Project Member Comment 15 by sheriffbot@chromium.org, Apr 28
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
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
Status: Fixed
Project Member Comment 20 by clusterf...@chromium.org, May 4
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.
Project Member Comment 21 by sheriffbot@chromium.org, May 4
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -ReleaseBlock-Beta
Labels: -Restrict-View-SecurityNotify
(removing restrict view since this was a trunk regression and has been fixed).
Labels: allpublic
Sign in to add a comment