Security: race condition lead to many fatal Errors in WebAssembly.validate

VULNERABILITY DETAILS
this is a similar issue as https://bugs.chromium.org/p/chromium/issues/detail?id=759624,  issue 759624  fixed WebAssembly.Module but missed WebAssembly.validate

VERSION
Chrome Version: [63.0.3239.84] + [stable]
Operating System: [all]

REPRODUCTION CASE
a poc is attached, just run "./d8 ./worker.js" to reproduce the crash. The d8 should be a debug version.

here is some crashes
crash 1:
#
# Fatal error in ../../v8/src/wasm/function-body-decoder-impl.h, line 290
# Debug check failed: kExprBrTable == decoder->read_u8<validate>(pc, "opcode") (14 vs. '54').
#

==== C stack trace ===============================

    /path/chromium/src/out/Debug/./libv8_libbase.so(v8::base::debug::StackTrace::StackTrace()+0x13) [0x7fa83255b693]
    /path/chromium/src/out/Debug/./libv8_libplatform.so(+0xdc1b) [0x7fa832536c1b]
    /path/chromium/src/out/Debug/./libv8_libbase.so(V8_Fatal(char const*, int, char const*, ...)+0xdc) [0x7fa832554c6c]
    /path/chromium/src/out/Debug/./libv8_libbase.so(+0x14a45) [0x7fa832554a45]
    /path/chromium/src/out/Debug/./libv8.so(+0xc94786) [0x7fa831f52786]
    /path/chromium/src/out/Debug/./libv8.so(+0xc9f7c0) [0x7fa831f5d7c0]
    /path/chromium/src/out/Debug/./libv8.so(+0xc99cca) [0x7fa831f57cca]
    /path/chromium/src/out/Debug/./libv8.so(v8::internal::wasm::VerifyWasmCode(v8::internal::AccountingAllocator*, v8::internal::wasm::WasmModule const*, v8::internal::wasm::FunctionBody&)+0x109) [0x7fa831f579d9]
    /path/chromium/src/out/Debug/./libv8.so(+0xc9a020) [0x7fa831f58020]
    /path/chromium/src/out/Debug/./libv8.so(+0xcdb4c9) [0x7fa831f994c9]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd7370) [0x7fa831f95370]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd2184) [0x7fa831f90184]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd1714) [0x7fa831f8f714]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd1338) [0x7fa831f8f338]
    /path/chromium/src/out/Debug/./libv8.so(v8::internal::wasm::SyncDecodeWasmModule(v8::internal::Isolate*, unsigned char const*, unsigned char const*, bool, v8::internal::wasm::ModuleOrigin)+0x47) [0x7fa831f906e7]
    /path/chromium/src/out/Debug/./libv8.so(v8::internal::wasm::SyncValidate(v8::internal::Isolate*, v8::internal::wasm::ModuleWireBytes const&)+0x4f) [0x7fa831f7520f]
    /path/chromium/src/out/Debug/./libv8.so(+0xd09034) [0x7fa831fc7034]
    /path/chromium/src/out/Debug/./libv8.so(+0x33d282) [0x7fa8315fb282]
    /path/chromium/src/out/Debug/./libv8.so(+0x43a115) [0x7fa8316f8115]
    /path/chromium/src/out/Debug/./libv8.so(+0x438209) [0x7fa8316f6209]
    /path/chromium/src/out/Debug/./libv8.so(+0x437c4d) [0x7fa8316f5c4d]
    [0x223aa4004384]
Received signal 4 ILL_ILLOPN 7fa832559ab2
Illegal instruction


crash 2:
#
# Fatal error in ../../v8/src/wasm/module-decoder.cc, line 650
# Debug check failed: !cmp_less(*it, *last).
#

==== C stack trace ===============================

    /path/chromium/src/out/Debug/./libv8_libbase.so(v8::base::debug::StackTrace::StackTrace()+0x13) [0x7fa6038b4693]
    /path/chromium/src/out/Debug/./libv8_libplatform.so(+0xdc1b) [0x7fa60388fc1b]
    /path/chromium/src/out/Debug/./libv8_libbase.so(V8_Fatal(char const*, int, char const*, ...)+0xdc) [0x7fa6038adc6c]
    /path/chromium/src/out/Debug/./libv8_libbase.so(+0x14a45) [0x7fa6038ada45]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd6dd1) [0x7fa6032eddd1]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd212b) [0x7fa6032e912b]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd1714) [0x7fa6032e8714]
    /path/chromium/src/out/Debug/./libv8.so(+0xcd1338) [0x7fa6032e8338]
    /path/chromium/src/out/Debug/./libv8.so(v8::internal::wasm::SyncDecodeWasmModule(v8::internal::Isolate*, unsigned char const*, unsigned char const*, bool, v8::internal::wasm::ModuleOrigin)+0x47) [0x7fa6032e96e7]
    /path/chromium/src/out/Debug/./libv8.so(v8::internal::wasm::SyncValidate(v8::internal::Isolate*, v8::internal::wasm::ModuleWireBytes const&)+0x4f) [0x7fa6032ce20f]
    /path/chromium/src/out/Debug/./libv8.so(+0xd09034) [0x7fa603320034]
    /path/chromium/src/out/Debug/./libv8.so(+0x33d282) [0x7fa602954282]
    /path/chromium/src/out/Debug/./libv8.so(+0x43a115) [0x7fa602a51115]
    /path/chromium/src/out/Debug/./libv8.so(+0x438209) [0x7fa602a4f209]
    /path/chromium/src/out/Debug/./libv8.so(+0x437c4d) [0x7fa602a4ec4d]
    [0x37c2b5d84384]
Received signal 4 ILL_ILLOPN 7fa6038b2ab2
Illegal instruction

Deleted: worker.js 796 bytes

worker.js 796 bytes View Download

Deleted: stack.wasm 335 bytes

stack.wasm 335 bytes Download

DCHECKS don't sound that severe. Waiting for security sheriffs to upload it to CF.

Looks like we are missing a memcpy of the bytes out of the buffer for the SyncValidate() method.

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5464777051865088.

Detailed report: https://clusterfuzz.com/testcase?key=5464777051865088

Job Type: linux_asan_d8_dbg
Crash Type: DCHECK failure
Crash Address: 
Crash State:
  !cmp_less(*it, *last) in module-decoder.cc
  v8::internal::wasm::ModuleDecoderImpl::DecodeExportSection
  v8::internal::wasm::ModuleDecoderImpl::DecodeSection
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=46985:46986

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5464777051865088

See https://github.com/google/clusterfuzz-tools for more information.

Re #3: As I understand it, the problem is that the DCHECK blocks clusterfuzz from knowing what happens in the absence of the DCHECK.

Detailed report: https://clusterfuzz.com/testcase?key=5464777051865088

Job Type: linux_asan_d8_dbg
Crash Type: DCHECK failure
Crash Address: 
Crash State:
  !cmp_less(*it, *last) in module-decoder.cc
  v8::internal::wasm::ModuleDecoderImpl::DecodeExportSection
  v8::internal::wasm::ModuleDecoderImpl::DecodeSection
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=46985:46986

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5464777051865088

See https://github.com/google/clusterfuzz-tools for more information.

I think we've treated other DCHECK triggers in d8 as High severity. Without the DCHECK it is possible this could lead to memory corruption in a sandboxed process.

I've also manually checked that the test case crashes builds going back to August.

titzer: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

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

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

commit f3c67392acb9804598cc23a142b31abdf4aeaac3
Author: Ben L. Titzer <titzer@chromium.org>
Date: Wed Jan 10 10:49:10 2018

[wasm] Improve copying behavior for SyncCompile and SyncValidate

This fixes a long-standing TODO to only make a copy of a module's
wire bytes if the input is a SharedArrayBuffer and also fixes the
concurrent-modification bug for synchronous validation.

R=clemensh@chromium.org
BUG= chromium:794091 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I8d2f20a9aeedbc306434853f8f6cfc070a24cf97
Reviewed-on: https://chromium-review.googlesource.com/856559
Commit-Queue: Ben Titzer <titzer@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50472}
[modify] https://crrev.com/f3c67392acb9804598cc23a142b31abdf4aeaac3/src/api.cc
[modify] https://crrev.com/f3c67392acb9804598cc23a142b31abdf4aeaac3/src/wasm/module-compiler.cc
[modify] https://crrev.com/f3c67392acb9804598cc23a142b31abdf4aeaac3/src/wasm/module-compiler.h
[modify] https://crrev.com/f3c67392acb9804598cc23a142b31abdf4aeaac3/src/wasm/wasm-js.cc
[modify] https://crrev.com/f3c67392acb9804598cc23a142b31abdf4aeaac3/test/fuzzer/wasm-async.cc

ClusterFuzz has detected this issue as fixed in range 50471:50472.

Detailed report: https://clusterfuzz.com/testcase?key=5464777051865088

Job Type: linux_asan_d8_dbg
Crash Type: DCHECK failure
Crash Address: 
Crash State:
  !cmp_less(*it, *last) in module-decoder.cc
  v8::internal::wasm::ModuleDecoderImpl::DecodeExportSection
  v8::internal::wasm::ModuleDecoderImpl::DecodeSection
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=46985:46986
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=50471:50472

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5464777051865088

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5464777051865088 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

Nice one!  The VRP panel decided to award $3,000 for this report :-)

This bug requires manual review: M65 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

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

[Bulk Edit]

+awhalley@ (Security TPM) for M65 merge review

govind@ - good for 65

Approving merge to M65 branch 3325 based on comment #28. Please merge ASAP so we can pick it up for next week Beta release. Thank you.

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

The fix was landed before the M65 branch point. Removing Merge-Approved label.

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