Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/ce67e40512a36401cb863ff78d59f7a28c127bf3 (Revert "Merge anonymous table boxes when appropriate.").

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

It's very likely that the suspected CL was the reason, but that was a revert, which means the problem was there before the origin CL https://chromium.googlesource.com/chromium/src/+/4766c5012f46eed5afec44dcbc10e7365eadbc1b

This issue is probably old.

I'm hitting Blink Reformat in all the contexts on the call stack so assigning it to the directory owners for quick triage.
If I find the culprit CL, I'll update the owner of the bug.

cbiesinger: 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

cbiesinger: Uh oh! This issue still open and hasn't been updated in the last 28 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

cbiesinger: Ping, can you please take a look when you have a chance? 

M-67 has sailed, but maybe we can get it in a stable update if we're lucky. This is our second-oldest High-severity bug (45 days), and we have a commitment to ship fixes to users in 60 days (https://chromium.googlesource.com/chromium/src/+/master/docs/security/severity-guidelines.md#High-severity).

Hello from your friendly security sheriff. This bug is now 54 days old, which is very near to exceeding the commitments we make publicly about our timelines fox fixing security bugs.

Is a better bisect needed to try and find the bug here? What needs to be done to move this bug onwards and upwards? Please update the bug as soon as possible.

Hi Rune, there hasn't been any progress on this, but it doesn't seem too difficult, even though it's an old and existing bug. Can you take a look? If not, please reassign to me.

I started on it here:

https://chromium-review.googlesource.com/c/chromium/src/+/852259

The first patch set is the change that fixed this issue but introduced others.

Are you sure this is a table bug? Just looking at the stack, it seems like an issue where we are forgetting to remove a LayoutObject from root_inline_box.h's floats_ when the LayoutObject is destroyed. Would it be possible to do more targeted temporary fix to squash the security bug?

READ of size 8 at 0x11c9f861d798 thread T0
#0 FloatingObject::FloatingObject
#1 FloatingObject::Create
#2 LayoutBlockFlow::InsertFloatingObject
#3 LayoutBlockFlow::LinkToEndLineIfNeeded
#4 LayoutBlockFlow::LayoutRunsAndFloats
#5 LayoutBlockFlow::LayoutInlineChildren

freed by thread T0 here:
#0 third_party/llvm/projects/compiler-rt/lib/asan/asan_malloc_win.cc:44
#1 LayoutBlockFlow::`scalar deleting destructor'
#2 LayoutObject::DestroyAndCleanupAnonymousWrappers
#3 Node::DetachLayoutTree
#4 ContainerNode::DetachLayoutTree
#5 Element::DetachLayoutTree

I'm able to reproduce the asan report with an asan build on Linux.

The reason I think it's related to tables is that my table change only changed anonymous correction for table layout and presumably fixed this issue.

I'm not familiar with inline layout or floats.

Morten, do you have any ideas?

I'll try revive my in-progress table correction and see if it still fixes this issue.

Nope, did not work.

First we need a reduction. After spending almost a day, at least I have something that fits on one page. :-P

Deleted: reduction-in-progress.html 1.1 KB

reduction-in-progress.html 1.1 KB View Download

Attaching a minimal testcase with explanations.

Deleted: tc.html 2.3 KB

tc.html 2.3 KB View Download

Hm, the test case could have been even more minimal; we don't need #emptySpan. All we have to do is take an inline-block with a float, and wrap it inside an anonymous block, and then remove the float. Requires a somewhat exotic inline layout situation, though. :)

I kicked off a clusterfuzz run with the minimized testcase to see if a different regression range is found: https://clusterfuzz.com/v2/testcase-detail/6095192256675840. Rune's patch may have been incorrectly blamed because the testcase had tables.

Windows ASAN64 is flaky, can you reupload to linux_asan_chrome_mp

Here's a clusterfuzz run on linux_asan_chrome_mp: https://clusterfuzz.com/v2/testcase-detail/5913320323022848

Thanks a lot for fixing this, Morten (hasn't landed yet, but still :-))

And thanks pdr for the fuzzer run. I can confirm that it was introduced by https://codereview.chromium.org/2828553002 (April 2017).

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

commit 17766b88aabf6c58a1019de65ca571afcaada2e1
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Thu Jun 21 12:17:07 2018

Line boxes hold potentially dangling pointers to FloatingObject.

And that's how it's supposed to work, apparently. We're just supposed to mark
such lines dirty when the objects get deleted, and the dangling pointers will
be cleaned up "soon enough".

When removing a FloatingObject, we need to mark the RootInlineBox object that
points to it as dirty. The case we were missing was the one where we remove ALL
floating objects. When removing just one, on the other hand, we use
LayoutBlockFlow::RemoveFloatingObject(), which already takes care of this.

Bug:  835613 
Change-Id: I2b978f1bb4c9a496a5dad4b097ef598630059a9d
Reviewed-on: https://chromium-review.googlesource.com/1108936
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569223}
[add] https://crrev.com/17766b88aabf6c58a1019de65ca571afcaada2e1/third_party/WebKit/LayoutTests/fast/block/float/remove-float-after-anonymous-block-insert-crash.html
[modify] https://crrev.com/17766b88aabf6c58a1019de65ca571afcaada2e1/third_party/blink/renderer/core/layout/layout_block_flow.cc

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

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

ClusterFuzz testcase 6167934377132032 appears to be flaky, updating reproducibility label.

This bug requires manual review: M68 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), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

I advise against merging this. This is not a very recent regression; see comment #c29.

Per #37, rejecting merge to 68. 

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