New issue
Advanced search Search tips

Issue 740023 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocking:
issue v8:6634



Sign in to add a comment

CHECK failure: 0 < len in decoder.h

Project Member Reported by ClusterFuzz, Jul 7 2017

Issue description

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

Fuzzer: inferno_js_fuzzer
Job Type: linux_cfi_d8
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  0 < len in decoder.h
  
Sanitizer: cfi (CFI)

Regressed: https://clusterfuzz.com/revisions?job=linux_cfi_d8&range=463294:463368

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


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: ahaas@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>WebAssembly
Owner: clemensh@chromium.org
Status: Assigned (was: Untriaged)
Labels: Restrict-View-SecurityTeam
Can this have security implication. ClusterFuzz didn't mark it as a security since it is a hard check failure in release builds, but maybe the state we get into is bad. If yes, please change to Type=Bug-Security.
Labels: -Restrict-View-SecurityTeam
Status: Started (was: Assigned)
This is a CHECK failure without security implications.
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 25 2017

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

commit 072d0e3eb62a54ab3608eb4ecd1155664a30878f
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Tue Jul 25 09:17:15 2017

[wasm] Allow for arbitrarily long error messages

We currently have a fixed limit of 256 characters for error messages
generated in the decoder. However, we sometimes embed names in it,
which makes it easy to generate a crash by using long names (e.g. for
exports) in invalid wasm modules.
This CL fixes this by switching to a stream based interface, allowing
to pass arbitrary objects to be printed. With this interface, we can
easily limit the length of output later.

R=titzer@chromium.org

Bug:  chromium:740023 
Change-Id: I2848c31c63a015157e2a3a9458b54e523060cd69
Reviewed-on: https://chromium-review.googlesource.com/565282
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46860}
[modify] https://crrev.com/072d0e3eb62a54ab3608eb4ecd1155664a30878f/src/wasm/decoder.h
[modify] https://crrev.com/072d0e3eb62a54ab3608eb4ecd1155664a30878f/src/wasm/function-body-decoder-impl.h
[modify] https://crrev.com/072d0e3eb62a54ab3608eb4ecd1155664a30878f/src/wasm/function-body-decoder.cc
[modify] https://crrev.com/072d0e3eb62a54ab3608eb4ecd1155664a30878f/src/wasm/module-decoder.cc
[modify] https://crrev.com/072d0e3eb62a54ab3608eb4ecd1155664a30878f/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/072d0e3eb62a54ab3608eb4ecd1155664a30878f/test/mjsunit/wasm/export-global.js

Blocking: v8:6634
Status: Fixed (was: Started)
Original issue fixed (we now allow for arbitrarily long error messages).
Opened another bug to actually truncate function names and module names in error messages, because we do not want the messages to grow arbitrarily big.
Project Member

Comment 7 by ClusterFuzz, Jul 26 2017

ClusterFuzz has detected this issue as fixed in range 489413:489455.

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

Fuzzer: inferno_js_fuzzer
Job Type: linux_cfi_d8
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  0 < len in decoder.h
  
Sanitizer: cfi (CFI)

Regressed: https://clusterfuzz.com/revisions?job=linux_cfi_d8&range=463294:463368
Fixed: https://clusterfuzz.com/revisions?job=linux_cfi_d8&range=489413:489455

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


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.
Project Member

Comment 8 by ClusterFuzz, Jul 26 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 5927058168610816 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Cc: titzer@chromium.org
Status: Started (was: Verified)
Reverting because of severe performance regressions, see https://crbug.com/749041.
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 31 2017

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

commit 20d25f403633f22b4fe1ee9b35a3a3b0ee3ba51f
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Mon Jul 31 13:00:54 2017

Revert "[wasm] Allow for arbitrarily long error messages"

This reverts commit 072d0e3eb62a54ab3608eb4ecd1155664a30878f.

Reason for revert: Performance regressions (https://crbug.com/749041).

Original change's description:
> [wasm] Allow for arbitrarily long error messages
> 
> We currently have a fixed limit of 256 characters for error messages
> generated in the decoder. However, we sometimes embed names in it,
> which makes it easy to generate a crash by using long names (e.g. for
> exports) in invalid wasm modules.
> This CL fixes this by switching to a stream based interface, allowing
> to pass arbitrary objects to be printed. With this interface, we can
> easily limit the length of output later.
> 
> R=​titzer@chromium.org
> 
> Bug:  chromium:740023 
> Change-Id: I2848c31c63a015157e2a3a9458b54e523060cd69
> Reviewed-on: https://chromium-review.googlesource.com/565282
> Reviewed-by: Ben Titzer <titzer@chromium.org>
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#46860}

TBR=titzer@chromium.org,clemensh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:740023 , chromium:749041
Change-Id: I005a60d55dcf01d350230f8d98f715bab9c43886
Reviewed-on: https://chromium-review.googlesource.com/593807
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47008}
[modify] https://crrev.com/20d25f403633f22b4fe1ee9b35a3a3b0ee3ba51f/src/wasm/decoder.h
[modify] https://crrev.com/20d25f403633f22b4fe1ee9b35a3a3b0ee3ba51f/src/wasm/function-body-decoder-impl.h
[modify] https://crrev.com/20d25f403633f22b4fe1ee9b35a3a3b0ee3ba51f/src/wasm/function-body-decoder.cc
[modify] https://crrev.com/20d25f403633f22b4fe1ee9b35a3a3b0ee3ba51f/src/wasm/module-decoder.cc
[modify] https://crrev.com/20d25f403633f22b4fe1ee9b35a3a3b0ee3ba51f/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/20d25f403633f22b4fe1ee9b35a3a3b0ee3ba51f/test/mjsunit/wasm/export-global.js

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 4 2017

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

commit ea82e09611fe415bc79b0bbae5a024a96c6f5373
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Fri Aug 04 09:20:34 2017

[wasm] Limit output length of user-provided strings

In order to limit the overall length of error message, limit the output
of string provided by the user. This is implemented by a helper class
which takes the maximum length as template argument and has simple
accessors for the start address and the length of the truncated string.

This is the compromise CL after
https://chromium-review.googlesource.com/c/566815 and
https://chromium-review.googlesource.com/c/594288.

R=titzer@chromium.org

Bug:  chromium:740023 , chromium:749041,  v8:6634 
Change-Id: I7c154eb18b3a6befd5ecabbd2f435b015ad71542
Reviewed-on: https://chromium-review.googlesource.com/600547
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47157}
[modify] https://crrev.com/ea82e09611fe415bc79b0bbae5a024a96c6f5373/src/wasm/module-compiler.cc
[modify] https://crrev.com/ea82e09611fe415bc79b0bbae5a024a96c6f5373/src/wasm/module-decoder.cc
[modify] https://crrev.com/ea82e09611fe415bc79b0bbae5a024a96c6f5373/src/wasm/wasm-module.h
[modify] https://crrev.com/ea82e09611fe415bc79b0bbae5a024a96c6f5373/test/mjsunit/wasm/export-global.js

Status: Fixed (was: Started)

Sign in to add a comment