New issue
Advanced search Search tips

Issue 729991 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Information Disclosure Issue in v8::wasm

Reported by xilinggg...@gmail.com, Jun 6 2017

Issue description

VULNERABILITY DETAILS

There is a out of boundary issue in v8::wasm::module-decoder, which will disclosure sensitive information in the heap.

In v8\src\wasm\module-decoder.cc, the function next(), DecodeModule(), and DecodeCustomSections() doesn't handle the custom section properly.

In next(), 
		if (section_code_ == kUnknownSectionCode &&
		  section_end_ > decoder_.pc()) {
		// skip to the end of the unknown section.
		uint32_t remaining =
			static_cast<uint32_t>(section_end_ - decoder_.pc());
		decoder_.consume_bytes(remaining, "section payload");
		// fall through and continue to the next section.
		} else {
		return;
		}
Here if section_end_ <= decoder_.pc, the iterator simply exit. The section_code_ remains be kUnknownSectionCode.

Then in function DecodeModule(),
		if (section_iter.more() && ok()) {
		      errorf(pc(), "unexpected section: %s",
                         SectionName(section_iter.section_code()));
		}
Because section_code_ is kUnknownSectionCode, here will not trigger the errorf and everything is OK.

Then in DecodeCustomSections(), all the data will be treated as checked, and simply run through.
So if we put a large section_length here, the payload_length will go out of the buffer boundary.

VERSION
Chrome Version: [59.0.3071.86] + [stable]
Operating System: [Windows 7 Service Pack1]

REPRODUCTION CASE
Open the attach wasm_exp.html, you'll see leaked heap values.
 
wasm_exp.html
498 bytes View Download
Components: Blink>JavaScript>WebAssembly
Cc: clemensh@chromium.org ahaas@chromium.org
Status: Available (was: Unconfirmed)
I can confirm the bug for v8 5.9.223.

The problem was fixed with https://chromium-review.googlesource.com/c/505490/, which landed in 6.0.247.
Owner: ahaas@chromium.org
Status: Assigned (was: Available)
Owner: clemensh@chromium.org
Status: Started (was: Assigned)
We are working on a fix for the 5.9 branch.
Cc: jochen@chromium.org
CL is here, to be merged into the 5.9 branch: https://chromium-review.googlesource.com/c/525754/

Jochen, PTAL.
And a regression test for the master branch: https://chromium-review.googlesource.com/c/525538/
Labels: M-59
What's the severity of the issue? Can you add Merge-Request labels and release block labels as appropriate please?
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/fa0d5be128ea9be4d72a0ff7d963dc118235af52

commit fa0d5be128ea9be4d72a0ff7d963dc118235af52
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Tue Jun 06 15:55:02 2017

[wasm] Add regression test

The regression is already fixed. This just adds a regression test to
ensure it will never be reintroduced.

R=ahaas@chromium.org
BUG= chromium:729991 

Change-Id: I5cf960cc756cbb7723041bc06a78d6a14c66e241
Reviewed-on: https://chromium-review.googlesource.com/525538
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45739}
[add] https://crrev.com/fa0d5be128ea9be4d72a0ff7d963dc118235af52/test/mjsunit/regress/wasm/regression-729991.js

Labels: Security_Severity-High Merge-Request-59 M-58
This bug allows creating an ArrayBuffer which contains a copy of the memory of user-controlled size, starting inside the SeqOneByteString which holds the wasm module bytes.
Thus it must be considered to allow reading more or less arbitrary memory of the process.

The bug already existed in M58, but since M59 is just being release, merging back seems not necessary.
Project Member

Comment 10 by sheriffbot@chromium.org, Jun 6 2017

Labels: -Merge-Request-59 Merge-Review-59 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Pri-1
Release block labels seem inappropriate, as the bug is already out there.
Labels: Security_Impact-Stable
Please tag with applicable OSs.
Labels: Arch-All OS-All
Cc: awhalley@chromium.org
Still waiting for official approval to land this CL directly on the 5.9 branch: https://chromium-review.googlesource.com/c/525754/
Project Member

Comment 17 by sheriffbot@chromium.org, Jun 7 2017

Labels: -M-58
Project Member

Comment 18 by sheriffbot@chromium.org, Jun 7 2017

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
Would somebody mind double checking what Chrome version v8 6.0.247 landed it? I get 60.0.3112.20 was released 6/6.  We should give this some more time to bake, but should certainly consider it for a 59 stable update if one occurs.
https://chromium.googlesource.com/chromium/src/+/92860763bac3133a6729756f0f7899f69c22e9b2 brought 6.0.249 to Chrome 60.0.3104.0. The prior roll was 6.0.242 for Chrome 60.0.3103.0.
6.0.249 (containing a refactoring which fixed this bug) was rolled into chrome on May 17. This is contained in 60.0.3104.0.

We don't want to merge that CL back though. It's much lower risk to just land the CL referenced in #16 on the 5.9 branch.
I am waiting for Jochen (or someone else) to approve this.
Cc: adamk@chromium.org
+ Adam Klein
So I see this issue had already been fixed in the DEV branch.

May I ask that whether can I still receive the CVE and reward?
Project Member

Comment 24 by sheriffbot@chromium.org, Jun 8 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -Merge-Review-59 Merge-Approved-59
Landing https://chromium-review.googlesource.com/c/525754/ on the v8 5.9 branch sounds good to me.
Project Member

Comment 26 by bugdroid1@chromium.org, Jun 9 2017

Labels: merge-merged-5.9
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e23b659865e16de9482e19732650d57b3ff0b374

commit e23b659865e16de9482e19732650d57b3ff0b374
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Fri Jun 09 09:12:47 2017

[wasm] Validate length of unknown sections

For empty unknown sections, the section iterator was just returning
without checking any succeeding sections. Instead, it should continue
explictly skipping over them, while checking that the payload length is
valid (i.e. in bounds of the module bytes).

R=ahaas@chromium.org, jochen@chromium.org
BUG= chromium:729991 
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Also-by: ahaas@chromium.org
Change-Id: Ia64068b40817a9da4cca836f1a21462265481a2b
Reviewed-on: https://chromium-review.googlesource.com/525754
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/branch-heads/5.9@{#71}
Cr-Branched-From: fe9bb7e6e251159852770160cfb21dad3cf03523-refs/heads/5.9.211@{#1}
Cr-Branched-From: 70ad23791a21c0dd7ecef8d4d8dd30ff6fc291f6-refs/heads/master@{#44591}
[modify] https://crrev.com/e23b659865e16de9482e19732650d57b3ff0b374/src/wasm/module-decoder.cc

Labels: -Merge-Approved-59
Labels: Release-1-M59
Labels: reward-topanel
Labels: CVE-2017-5088
Labels: -reward-topanel reward-4000
Congratulations! The VRP Panel decided to award $4,000 for this report!
Project Member

Comment 32 by sheriffbot@chromium.org, Sep 14 2017

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
Labels: reward-unpaid
Labels: -reward-unpaid reward-inprocess
Labels: CVE_description-submitted

Sign in to add a comment