aboxhall@: Your CL has caused a security regression -- https://chromium.googlesource.com/chromium/src/+/58a3e83e1325c8a366941564ca8ce096bb1345df

It appears that when AXObjectImpl::AccessibilityIsIgnored() calls UpdateDistribution() on its Document's owner, that can cause modifications to the layout tree, including destruction of a LayoutText object that is higher up in the stack.

Can you please have a look?

Looking at this now.

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

commit 6281b70b9a36a64590f99c799ee55264da6cb52d
Author: aboxhall <aboxhall@chromium.org>
Date: Thu Jun 01 10:09:50 2017

Check whether a text node may be about to be redistributed in ShouldUpdateLayoutByReattaching(). Prevents use-after-free when SetTextWithOffset is called with dirty distribution.

BUG= 726716 

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

[modify] https://crrev.com/6281b70b9a36a64590f99c799ee55264da6cb52d/third_party/WebKit/Source/core/dom/Text.cpp

ClusterFuzz has detected this issue as fixed in range 476239:476249.

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

Fuzzer: bj_broddelwerk
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x60e0000c0780
Crash State:
  blink::LayoutText::SetText
  blink::LayoutText::SetTextWithOffset
  blink::Text::UpdateTextLayoutObject
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=474583:474657
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=476239:476249

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


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 5713784797921280 is verified as fixed, so closing issue.

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

Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

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

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

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

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

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

aboxhall@ - mind doing the merge to M60?  Thanks!

I'll merge now

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

commit 986255635c4af811abecf049b18c0b978d3a5703
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Jul 06 20:15:33 2017

Merge to M60: Check whether a text node may be about to be redistributed in ShouldUpdateLayoutByReattaching(). Prevents use-after-free when SetTextWithOffset is called with dirty distribution.

BUG= 726716 

Review-Url: https://codereview.chromium.org/2913133002
Cr-Original-Commit-Position: refs/heads/master@{#476242}
Review-Url: https://codereview.chromium.org/2967423002 .
Cr-Commit-Position: refs/branch-heads/3112@{#533}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/986255635c4af811abecf049b18c0b978d3a5703/third_party/WebKit/Source/core/dom/Text.cpp

Thanks dmazzoni@!

Isn't the CL merged in #20 the one that caused perf regression and we chose do different fix?

I believe kochi@ merged the Alice's newer one already that this merge isn't necessary, no?

This is the merge CL:
https://codereview.chromium.org/2957733002

So I think 3112@{#464} fixed this problem in better way, and in #20, 3112@{#533} has an old fix that has a perf issue.

Should we revert #20 I guess?

Ah, you're right - the correct fix was from  bug 729229  and this shouldn't have been marked for merge.

Reverting now.

Thanks! Pardon the confusion.

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