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 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security

Blocking:
issue 766253



Sign in to add a comment
link

Issue 766260: Security: WebAsm OOB ArrayBuffer

Reported by mea...@chromium.org, Sep 18 2017 Project Member

Issue description

Split from  bug 766253 :

"""
WebAsm instance builder reads imports from an attacker-controlled object in v8/src/wasm/wasm-module.cc:1625 ProcessImports(). Imports can be getters, which run while the instance is being built and is not in a consistent state. If the getter builds another instance for the same module, then the instances will share a WasmCompiledModule, but will have different ArrayBuffers for memory. Compiled module will reference one memory buffer. If the second memory grows, then the compiled module gets confused and relocates to OOB memory. For trunk, the code has moved to wasm/module-compiler.cc. Exploit in wasm_xpl.js.
"""
 
wasm_xpl.js
7.7 KB View Download

Comment 1 by mea...@chromium.org, Sep 18 2017

Blocking: 766253

Comment 2 by bradnelson@chromium.org, Sep 18 2017

Cc: gdeepti@chromium.org bradnelson@chromium.org mtrofin@chromium.org
Owner: eholk@chromium.org
Eric, Mircea, Deepti, can you take a look at this.

Comment 3 by mea...@chromium.org, Sep 18 2017

Cc: gzo...@gmail.com
+gzobqq@gmail.com, the original reporter.

Comment 4 by mtrofin@chromium.org, Sep 18 2017

Can we get access to the quoted 766253?

Comment 5 by mea...@chromium.org, Sep 18 2017

Yes, I CCed you.

Comment 6 by mtrofin@chromium.org, Sep 18 2017

Owner: mtrofin@chromium.org
Status: Started (was: Unconfirmed)

Comment 7 by awhalley@google.com, Sep 18 2017

Cc: awhalley@chromium.org

Comment 8 by mtrofin@chromium.org, Sep 19 2017

I assume we want this backported to 60 and 61, as well as 62, correct?

Comment 9 by bugdroid1@chromium.org, Sep 19 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1bc42ab44cd62b860153249e281e8d3b40314cc3

commit 1bc42ab44cd62b860153249e281e8d3b40314cc3
Author: Mircea Trofin <mtrofin@chromium.org>
Date: Tue Sep 19 02:45:11 2017

[wasm] Sanitize imports

Sanitize imports before we start the instance building process. This
avoids the possibility of exiting to JS while building instances,
and allowing JS to observe an inconsistent state of the wasm world -
e.g. incomplete specialization chains.

We now validate we never exit to JS during that process.

Bug:  chromium:766260 

Change-Id: I34930c8b70bdac16af464b3f62a2b6a38107acb3
Reviewed-on: https://chromium-review.googlesource.com/671480
Commit-Queue: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48074}
[modify] https://crrev.com/1bc42ab44cd62b860153249e281e8d3b40314cc3/src/wasm/module-compiler.cc
[modify] https://crrev.com/1bc42ab44cd62b860153249e281e8d3b40314cc3/src/wasm/module-compiler.h
[modify] https://crrev.com/1bc42ab44cd62b860153249e281e8d3b40314cc3/src/wasm/wasm-objects.cc
[modify] https://crrev.com/1bc42ab44cd62b860153249e281e8d3b40314cc3/src/wasm/wasm-objects.h

Comment 10 by mtrofin@chromium.org, Sep 19 2017

Labels: Merge-Request-62 Merge-Request-61 Merge-Request-60

Comment 11 by gov...@chromium.org, Sep 19 2017

Pls apply appropriate OSs label.

Comment 12 by mtrofin@chromium.org, Sep 19 2017

Labels: OS-Android OS-Chrome OS-Fuchsia OS-iOS OS-Linux OS-Mac OS-Windows

Comment 13 by sheriffbot@chromium.org, Sep 19 2017

Project Member
Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: Less than 24 days to go before AppStore submit on M62
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 14 by gov...@chromium.org, Sep 19 2017

+  awhalley@ for merge review

Comment 15 by sheriffbot@chromium.org, Sep 19 2017

Project Member
Labels: M-61

Comment 16 by sheriffbot@chromium.org, Sep 19 2017

Project Member
Labels: Pri-1

Comment 17 by sheriffbot@chromium.org, Sep 19 2017

Project Member
Status: Fixed (was: Started)
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

Comment 18 by gov...@chromium.org, Sep 19 2017

Cc: keta...@chromium.org cma...@chromium.org josa...@chromium.org abdulsyed@chromium.org amineer@chromium.org
+ Release TPMs as FYI (awhalley@ is currently looking).

Comment 19 by awhalley@chromium.org, Sep 19 2017

Note that this (and the other bugs split from  issue 766253 ) were responsibly disclosed, so while these should be picked up if we respin for other issues I don't consider them a driver for a respin.

Comment 20 by abdulsyed@chromium.org, Sep 19 2017

Labels: -Merge-Review-62 Merge-Approved-62
Approving this for M62 (branch: 3202)

Comment 21 by mtrofin@chromium.org, Sep 19 2017

Labels: -Merge-Approved-62
M62 merge done: https://chromium-review.googlesource.com/c/v8/v8/+/673549

Comment 22 by sheriffbot@chromium.org, Sep 20 2017

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

Comment 23 Deleted

Comment 24 by keta...@chromium.org, Sep 21 2017

Labels: -Merge-Request-61 Merge-Approved-61
Approving merge to M61.

Comment 25 by awhalley@google.com, Sep 21 2017

Labels: -Merge-Approved-61 Merge-Review-61
ketakid@ -

Comment 26 by awhalley@google.com, Sep 21 2017

ketakid@ - this has had very little bake time, I'd be very hesitant to take in 61, is there a particular driver?

(and sorry if the reason was my (now deleted) comment 23, which was intended for a different bug)

Comment 27 by mtrofin@chromium.org, Sep 25 2017

Folks, are we ready to merge this in M61?

Comment 28 by josa...@chromium.org, Sep 25 2017

Labels: -Merge-Request-60
Removing request for M60 since there are no more M60 planned releases

Ketaki to confirm on M61 merge timing/approval

Comment 29 by josa...@chromium.org, Oct 6 2017

Labels: -Merge-Review-61 Merge-Rejected-61
What is the business/urgent case to merge this? I'd prefer to target M-62 which is targeted few weeks from now 

Feel free to re request if there is a very strong reason that can't wait few weeks

Comment 30 by awhalley@google.com, Oct 6 2017

Labels: -M-61 M-62

Comment 31 by mtrofin@chromium.org, Oct 6 2017

The only source of urgency is the security aspect - a malicious program can access random data in the same process (renderer).

M62 has it already, see #21.

Comment 32 by josa...@chromium.org, Oct 6 2017

Let's target M62

Comment 33 by titzer@chromium.org, Oct 9 2017

Cc: hablich@chromium.org
+hablich

I don't understand why this was merge rejected from 61. This is a security vulnerability that I believe affects 61 as well.

Comment 34 by hablich@chromium.org, Oct 12 2017

See https://bugs.chromium.org/p/chromium/issues/detail?id=766260#c19. Retrospectively I would also have preferred to merge this but not respin for it.

We are branching for 6.3 today so 6.1 is obsolete.

Comment 35 by awhalley@chromium.org, Nov 6 2017

Labels: CVE-2017-15401

Comment 36 by mnissler@chromium.org, Nov 13 2017

Labels: -Restrict-View-SecurityNotify

Comment 37 by mnissler@chromium.org, Nov 13 2017

Labels: allpublic

Comment 38 by sheriffbot@chromium.org, Mar 27 2018

Project Member
Labels: -M-62 M-65

Comment 39 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-missing

Comment 40 by awhalley@chromium.org, Jan 4

Labels: -CVE_description-missing CVE_description-submitted

Sign in to add a comment