rockot, I initially assumed this was an IndexDB bug but I'm wondering if it might be a more general Mojo serialization/deserialization issue (https://chromium-review.googlesource.com/528556 is in the regression range). Could you take a look? Feel free to unassign yourself if you think it's unrelated.

This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

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

 Issue 733651  has been merged into this issue.

Adding some more components/cc's for help in figuring out if this is a mojo issue or an IndexedDB issue.

So, how does ASAN read overflow detection work exactly? Does it rely on shadow memory + hooks into common operations?

From the stack this looks to be happening within the StructTraits for content::IndexedDBKey:

https://cs.chromium.org/chromium/src/content/common/indexed_db/indexed_db_struct_traits.cc?rcl=2b36032776688471227ab1ebd9cb21faeb8e9bf4&l=77

The buffer overflow appears to be in the string constructor there, presumably while reading from the |binary| vector's data. This does not make sense, because the constructor arguments are obviously safe.

Is it possible that there is a buffer overflow in data_view.ReadBinary, but ASAN doesn't detect it until the bad data (copied into |binary|) is copied again from within the string constructor? Seemed kind of stretch to me at first, but I'm failing to imagine any more reasonable explanations.

(oops)

Still repros locally with my change reverted, so I'm ruling that out. There are no other Mojo changes in the regression range.

Since I have an environment set up I'm going to keep bisecting. There are a few other potential culprits worth considering.

I have bisected this locally to a clang roll: https://chromium.googlesource.com/chromium/src/+/3a26e05c70e63815c7c18284e5e006b9d0ac75af

Not sure who should look at this, so assigning to hans@ and CC+thakis@

Does this still repro after #480009?

Trying now.

Still repros at r480009

Taking a look. The shadow memory looks like this:

Shadow bytes around the buggy address:
0x0c0a8009eea0: fa fa fa fa fa fa fa fa fd fd fa fa fa fa fa fa
0x0c0a8009eeb0: fa fa fd fd fa fa fa fa fa fa fa fa fd fa fa fa
0x0c0a8009eec0: fa fa fa fa fa fa fd fa fa fa fa fa fa fa fa fa
0x0c0a8009eed0: fd fa fa fa fa fa fa fa fa fa fd fd fa fa fa fa
0x0c0a8009eee0: fa fa fa fa fd fa fa fa fa fa fa fa fa fa 00 fa
=>0x0c0a8009eef0: fa fa fa fa fa fa fa fa[01]fa fa fa fa fa fa fa
0x0c0a8009ef00: fa fa fd fa fa fa fa fa fa fa fa fa fd fd fa fa
0x0c0a8009ef10: fa fa fa fa fa fa fd fd fa fa fa fa fa fa fa fa
0x0c0a8009ef20: fd fd fa fa fa fa fa fa fa fa fd fa fa fa fa fa
0x0c0a8009ef30: fa fa fa fa fd fd fa fa fa fa fa fa fa fa fd fa
0x0c0a8009ef40: fa fa fa fa fa fa fa fa fd fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable:           00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone:       fa
Freed heap region:       fd
Stack left redzone:      f1
Stack mid redzone:       f2
Stack right redzone:     f3
Stack after return:      f5
Stack use after scope:   f8
Global redzone:          f9
Global init order:       f6
Poisoned by user:        f7
Container overflow:      fc
Array cookie:            ac
Intra object redzone:    bb
ASan internal:           fe
Left alloca redzone:     ca
Right alloca redzone:    cb


Perhaps its a load that got widened somehow.

rockot: How did you reproduce? When I download the testcase from clusterfuzz, all I get is a file that looks like this:

<script src="../../resources/js-test.js"></script>
<script src="resources/shared.js"></script>
<script src="resources/key-type-binary.js"></script>

Clusterfuzz bug prevents the testcase from being easily reproable - I worked with a clusterfuzz eng to sort it out. (Will ping privately)

Unzipping this in your chromium src should give you what you need

Then you can repro by loading third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary-private.html

Deleted: repro.zip 1.3 KB

repro.zip 1.3 KB Download

Bisection points to this LLVM commit:

----
Author: evstupac
Date: Tue Jun  6 15:04:16 2017
New Revision: 304824

URL: http://llvm.org/viewvc/llvm-project?rev=304824&view=rev
Log:
Fix PR23384 (part 3 of 3)

Summary:
The patch makes instruction count the highest priority for
 LSR solution for X86 (previously registers had highest priority).

Reviewers: qcolombet

Differential Revision: http://reviews.llvm.org/D30562

From: Evgeny Stupachenko <evstupac@gmail.com>
----

Maybe that uncovered a bug in loop strength reduce (LSR) or asan..





Dumping some notes.. this was the command I bisected on:

$ ninja -C build.release clang && touch /work/chromium/src/content/common/indexed_db/indexed_db_struct_traits.cc && ninja -C /work/chromium/src/out/asan chrome && ASAN_OPTIONS=redzone=64:symbolize=1:detect_stack_use_after_return=1:alloc_dealloc_mismatch=0:detect_leaks=0:print_scariness=1:check_malloc_usable_size=0:max_uar_stack_size_log=16:use_sigaltstack=1:handle_abort=1:strict_memcmp=0:detect_container_overflow=1:coverage=0:detect_odr_violation=0:allocator_may_return_null=1:handle_sigill=1:handle_segv=1:fast_unwind_on_fatal=1 /work/chromium/src/out/asan/chrome /work/chromium/src/third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary-private.html

$ cat /work/chromium/src/out/asan/args.gn 
# Build arguments go here.
# See "gn args <out_dir> --list" for available build arguments.
enable_ipc_fuzzer = true
is_asan = true
is_component_build = false
is_debug = false
is_lsan = true
sanitizer_coverage_flags = "trace-pc-guard"
strip_absolute_paths_from_debug_symbols = true
v8_enable_verify_heap = true
llvm_force_head_revision=true clang_use_chrome_plugins=false clang_base_path="/work/llvm.combined/build.release"

I haven't been able to create a stand-alone compiler test case from this, and I won't get any further today.

Starting to get a stand-alone repro:

#include <string>
#include <vector>
using namespace std;

struct DataView {
  DataView() {}
  bool ReadBinary(std::vector<uint8_t> *v) const {
    v->push_back(1);
    return true;
  }
};

void g(const std::string &s) {
  volatile void* p = (volatile void*)&s;
}

bool f(const DataView &data_view) {
  std::vector<uint8_t> binary;
  if (!data_view.ReadBinary(&binary))
      return false;
  g(std::string(binary.data(), binary.data() + binary.size()));
  return true;
}

int main() {
  DataView d;
  f(d);
  return 0;
}


$ ../../third_party/llvm-build/Release+Asserts/bin/clang -fsanitize=address -nostdinc++ -isystem../../buildtools/third_party/libc++/trunk/include -isystem../../buildtools/third_party/libc++abi/trunk/include /tmp/a.cc ./libc++.so -Wl,-rpath=\$ORIGIN/. -O2 -fno-exceptions -march=x86-64 -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -fsanitize-coverage=trace-pc-guard -mllvm -sanitizer-coverage-prune-blocks=1 -fsanitize=address -fsanitize-address-use-after-scope -g1  && ASAN_OPTIONS=symbolize=1 ./a.out

Shorter command-line:

$ ../../third_party/llvm-build/Release+Asserts/bin/clang -fsanitize=address -fsanitize-coverage=trace-pc-guard -O2 -g1 -nostdinc++ -isystem../../buildtools/third_party/libc++/trunk/include -isystem../../buildtools/third_party/libc++abi/trunk/include /tmp/a.cc ./libc++.so -Wl,-rpath=\$ORIGIN/. && ASAN_OPTIONS=symbolize=1 ./a.out

LLVM bug: https://bugs.llvm.org/show_bug.cgi?id=33514

ClusterFuzz has detected this issue as fixed in range 480776:480824.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_asan_chrome_mp
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x605000497bd1
Crash State:
  indexed_db::mojom::DatabaseStubDispatch::Accept
  mojo::InterfaceEndpointClient::HandleValidatedMessage
  IPC::ChannelAssociatedGroupController::Accept
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=479114:479249
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=480776:480824

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


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs 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 4741672201355264 is verified as fixed, so closing issue.

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

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