ahaas@, this is a crash we've discussed over hangouts yesterday. Please feel free to close if it's not a valid issue.

Hi Deepti,

The problem in this issue is the bounds check of a store instruction after a grow_memory instruction. The store instruction has an offset which is bigger than the memory size after growing and an address X which is less than the number of bytes the memory was extended by grow_memory, e.g. X = 16 and grow_memory grows memory by 1 page. In such a scenario the bounds check does not work, and the out-of-bounds memory access causes a segfault.

Can you fix this bug?

ClusterFuzz has detected this issue as fixed in range 417976:418090.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6044935822508032

Fuzzer: libfuzzer_v8_wasm_code_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Fatal error
Crash Address: 
Crash State:
  v8::Isolate::Dispose
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_ubsan&range=415619:415673
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_ubsan&range=417976:418090

Minimized Testcase (0.01 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96WMRK7wPGaW1jghsj2aNwQZuuGFZYaokY-KdMiCsRUM1O1yRwIbzhYpnIm4ePOj4Ce51s00G5C4Ij7HceukQzOY5qULkz49cOQIogLyA4FIV2soYOnMWmH0PgqF3uMP2gMFi5x2Y8N-BTRGMe9mH9VSYrxWg?testcase_id=6044935822508032

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md 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 is verified as fixed, closing issue.

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

Reopening this issue because the bounds check for the scenario described by ahaas@ is incorrect. I'm assuming this got closed because this particular test case could be flaky. A more reliable reduced JS test that currently fails and should pass - 

function testGrowMemoryOutOfBoundsOffset() {
  var builder = genGrowMemoryBuilder();
  builder.addMemory(1, 1, false);
  var module = builder.instantiate();
  var offset, val;
  function peek() { return module.exports.load(offset); }
  function poke(value) { return module.exports.store(offset, value); }
  function growMem(pages) { return module.exports.grow_memory(pages); }

  offset = 3*kPageSize + 4;
  assertTraps(kTrapMemOutOfBounds, poke);

  assertEquals(1, growMem(1));
  assertTraps(kTrapMemOutOfBounds, poke);

  assertEquals(2, growMem(1));
  assertTraps(kTrapMemOutOfBounds, poke);
}

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

commit 64e43be959630cd0640d9ddb92ffdc9511679410
Author: gdeepti <gdeepti@chromium.org>
Date: Wed Sep 28 20:55:42 2016

Fix bounds check of a store instruction after a grow_memory instruction
 - Store instruction with an offset bigger than GrowMemory offset should handle out of bounds correctly
 - Refactor to separate runnning from compile so arguments can be passed in to module builder tests.

BUG= chromium:644670 

R=ahaas@chromium.org, titzer@chromium.org

Review-Url: https://codereview.chromium.org/2373613004
Cr-Commit-Position: refs/heads/master@{#39840}

[modify] https://crrev.com/64e43be959630cd0640d9ddb92ffdc9511679410/src/compiler/wasm-compiler.cc
[modify] https://crrev.com/64e43be959630cd0640d9ddb92ffdc9511679410/test/cctest/wasm/test-run-wasm-module.cc
[modify] https://crrev.com/64e43be959630cd0640d9ddb92ffdc9511679410/test/common/wasm/wasm-module-runner.cc
[modify] https://crrev.com/64e43be959630cd0640d9ddb92ffdc9511679410/test/common/wasm/wasm-module-runner.h
[modify] https://crrev.com/64e43be959630cd0640d9ddb92ffdc9511679410/test/mjsunit/wasm/grow-memory.js

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

commit e6071a9c067e07304329250e040b652fb82e9b5e
Author: titzer <titzer@chromium.org>
Date: Thu Sep 29 18:03:46 2016

[wasm] Remove improper assembler check for grow memory.

Note that the offset can still be out of bounds, even after grow memory. The calculation of the remaining size can overflow.

R=gdeepti@chromium.org
BUG= chromium:644670 

Review-Url: https://codereview.chromium.org/2376153003
Cr-Commit-Position: refs/heads/master@{#39886}

[modify] https://crrev.com/e6071a9c067e07304329250e040b652fb82e9b5e/src/assembler.cc
[modify] https://crrev.com/e6071a9c067e07304329250e040b652fb82e9b5e/test/mjsunit/wasm/grow-memory.js

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

commit a2a9b4a7eaa56908c4611b66a87590dfb917f3be
Author: adamk <adamk@chromium.org>
Date: Thu Sep 29 21:54:35 2016

Revert of [wasm] Remove improper assembler check for grow memory. (patchset #2 id:20001 of https://codereview.chromium.org/2376153003/ )

Reason for revert:
grow-memory test now fails on Linux dbg, blocking the CQ:

https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20debug/builds/11217

Original issue's description:
> [wasm] Remove improper assembler check for grow memory.
>
> Note that the offset can still be out of bounds, even after grow memory. The calculation of the remaining size can overflow.
>
> R=gdeepti@chromium.org
> BUG= chromium:644670 
>
> Committed: https://crrev.com/e6071a9c067e07304329250e040b652fb82e9b5e
> Cr-Commit-Position: refs/heads/master@{#39886}

TBR=gdeepti@chromium.org,titzer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:644670 

Review-Url: https://codereview.chromium.org/2378973003
Cr-Commit-Position: refs/heads/master@{#39889}

[modify] https://crrev.com/a2a9b4a7eaa56908c4611b66a87590dfb917f3be/src/assembler.cc
[modify] https://crrev.com/a2a9b4a7eaa56908c4611b66a87590dfb917f3be/test/mjsunit/wasm/grow-memory.js

 Issue v8:5529  has been merged into this issue.

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

commit e936685b0d15a246a9ce1ef64e0e7f8c01ca186e
Author: gdeepti <gdeepti@chromium.org>
Date: Thu Oct 20 07:38:55 2016

[wasm] Remove incorrect assembler check.

BUG= chromium:644670 
R=ahaas@chromium.org, titzer@chromium.org

Review-Url: https://chromiumcodereview.appspot.com/2438593003
Cr-Commit-Position: refs/heads/master@{#40451}

[modify] https://crrev.com/e936685b0d15a246a9ce1ef64e0e7f8c01ca186e/src/assembler.cc
[modify] https://crrev.com/e936685b0d15a246a9ce1ef64e0e7f8c01ca186e/test/mjsunit/wasm/grow-memory.js

Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

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

We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.