Thanks for the report. This crashes the renderer on stable, and also on HEAD on Linux in my testing.

yhirano and ricea, can you please follow this up?

ricea@, can you take a look?

In my environment, ToT crashes in V8ScriptRunner::CallExtra in Body::IsBodyUsedForDcheck. When I disabled the DCHECK the crash went away. So it's possible that the reported crash is one of the crashes you fixed previously.

Pretty sure I fixed this in https://chromium-review.googlesource.com/c/chromium/src/+/1235779, but I'm checking now.

Confirmed fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1235779.

Simplified repro:

<script>
  Promise.prototype.constructor = 1;
  fetch('/aaaaaaa.jpg', { method: "POST", body: 1});
</script>

Incidentally, the crash when DCHECK is enabled is a known issue. Unfortunately the DCHECKs can have side-effects in exceptional situations, and there's no way around it except to remove the DCHECKs. This is not a problem for release builds.

Does this need a merge to M70?

#10 My personal view is that we don't. I don't know a way to exploit a write to address 0x80, and the crash rate doesn't look too bad to me: https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3AFetchManager%3A%3AFetch%27

I don't think it justifies a late-beta merge, but I am interested in other opinions.

I'd like to push for a merge in the interests of stability - it looks like a pretty safe patch, and anything that reduces crashes is a good thing.

Yes, it's a safe merge (as far as any merge can be safe). I'm happy to do it if you want it.

This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

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

branch:3538

Thanks for going for the merge. :)

awhalley: I think this was fixed prior to the bug being reported (at least, the CL was already written before the bug was reported, but there's insufficient timestamp granularity for me to tell if it landed before the bug was reported) - not sure if that qualifies it for reward-topanel?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5177016f67e94273def90f9c8cede81aeeff2a0d

commit 5177016f67e94273def90f9c8cede81aeeff2a0d
Author: Adam Rice <ricea@chromium.org>
Date: Tue Oct 02 09:24:28 2018

Check ExceptionState after PassRequestData

In blink::GlobalFetchImpl::Fetch(), ExceptionState was not checked after
calling PassRequestData(). Since it can fail and return null, this led
to segmentation faults.

Bail out early if an exception was thrown.

BUG=882841, 890023 
TBR=ricea@chromium.org

(cherry picked from commit 0ca4e6f52c6924bc6789afe356737365224409dd)

Change-Id: I5317560d070df63f97afe78ec9997cfc0ebd92e0
Reviewed-on: https://chromium-review.googlesource.com/1235779
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#594592}
Reviewed-on: https://chromium-review.googlesource.com/1256405
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#805}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/5177016f67e94273def90f9c8cede81aeeff2a0d/third_party/blink/renderer/core/fetch/global_fetch.cc

Hi pawel@ - the VRP panel look a look at this and deemed it not to be exploitable, I'm afraid.

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