Issue metadata
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.
,
Jun 6 2017
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.
,
Jun 6 2017
,
Jun 6 2017
We are working on a fix for the 5.9 branch.
,
Jun 6 2017
CL is here, to be merged into the 5.9 branch: https://chromium-review.googlesource.com/c/525754/ Jochen, PTAL.
,
Jun 6 2017
And a regression test for the master branch: https://chromium-review.googlesource.com/c/525538/
,
Jun 6 2017
What's the severity of the issue? Can you add Merge-Request labels and release block labels as appropriate please?
,
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
,
Jun 6 2017
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.
,
Jun 6 2017
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
,
Jun 6 2017
Release block labels seem inappropriate, as the bug is already out there.
,
Jun 6 2017
,
Jun 6 2017
Please tag with applicable OSs.
,
Jun 6 2017
,
Jun 6 2017
,
Jun 7 2017
Still waiting for official approval to land this CL directly on the 5.9 branch: https://chromium-review.googlesource.com/c/525754/
,
Jun 7 2017
,
Jun 7 2017
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
,
Jun 7 2017
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.
,
Jun 7 2017
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.
,
Jun 7 2017
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.
,
Jun 7 2017
+ Adam Klein
,
Jun 8 2017
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?
,
Jun 8 2017
,
Jun 8 2017
Landing https://chromium-review.googlesource.com/c/525754/ on the v8 5.9 branch sounds good to me.
,
Jun 9 2017
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
,
Jun 9 2017
,
Jun 9 2017
,
Jun 12 2017
,
Jun 12 2017
,
Jun 15 2017
Congratulations! The VRP Panel decided to award $4,000 for this report!
,
Sep 14 2017
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
,
Oct 6 2017
,
Oct 6 2017
,
Apr 25 2018
|
|||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||
Comment 1 by elawrence@chromium.org
, Jun 6 2017