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

Most likely caused by https://chromium.googlesource.com/chromium/src/+/f2e52582169d42ef9a3f836b8125c755ce7b1eee

aleventhal: Can you PTAL?

hello, can we please get an update on this bug?

I've created a revert in https://chromium-review.googlesource.com/c/chromium/src/+/1103097 - assuming this passes CQ, I'll land later today.

Unfortunately, this did not revert cleanly.

Please address this bug as soon as possible.

CL in #4 has been in Chrome since 69.0.3447.0 so this is now in Dev channel. This regression should not reach Beta channel.

Security bugs are important, and this needs to be looked at - the CL needs to be reverted or the bug needs diagnosing by the team.

Please look at this as a matter of urgency.

M69 branch is coming soon on July 19th, Your bug is marked as ReleaseBlock-Beta for M69. Please try to land the fix ASAP to trunk in order to prevent many merges going after M69 branch. This will also help us to branch M69 from high quality trunk. Thank you.

It's looking like the true problem goes much farther back.
Reverting the change fixes the leak but we get a DCHECK failure instead.

I put an easier-to-read test case in https://chromium-review.googlesource.com/c/chromium/src/+/1105069
What happens is that the last <option> is removed from a <select> inside of a shadow root, although a couple of other things need to occur -- strangely, they are removal of a text node before the select, and document.execCommand("")

The root of the problem is that AXObject::AccessibilityIsIgnored() calls a couple of things that can cause a layout object to be detached:
- Node::UpdateDistributionForFlatTreeTraversal();
- AXObject::UpdateCachedAttributeValuesIfNeeded() --> AXObject::ComputeIsInertOrAriaHidden() --> Node::IsInert() --> FlatTreeTraversal::ParentElement()
Removing both of these is one way to fix the issue.

This may just be a yet-underscovered crash case that the activedescendant CL made apparent. We'll need too look for the true original regression window, or perhaps someone who understands this reentrant code can comment.

Attached crash.html which crashes in Chrome 67.

Deleted: crash.html 1.0 KB

crash.html 1.0 KB View Download

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5814216637022208.

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=6377166590443520.

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=6246119890485248.

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

Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x120356623ac0
Crash State:
  blink::AXObjectCacheImpl::GetOrCreate
  blink::AXObjectCacheImpl::HandleUpdateActiveMenuOption
  blink::LayoutMenuList::UpdateFromElement
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=563343:563345

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

See https://github.com/google/clusterfuzz-tools for more information.

finally got a repro from CF, but it still points at the same CL range.

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

commit a31413b0b94e734148b39636a105146141343e76
Author: Aaron Leventhal <aleventhal@chromium.org>
Date: Wed Jun 20 01:02:29 2018

Leak/crash with <option> removed from shadow tree

Bug:  848617 
Change-Id: I42b5b23a8497912128462ce83dacfd8009ef8797
Reviewed-on: https://chromium-review.googlesource.com/1105069
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568673}
[add] https://crrev.com/a31413b0b94e734148b39636a105146141343e76/third_party/WebKit/LayoutTests/accessibility/option-removed-from-shadow-dom-crash.html
[modify] https://crrev.com/a31413b0b94e734148b39636a105146141343e76/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc

@wfh were you able to manually repro the "oh snap" page in 67 with the attached file and --force-renderer-accessibility ?

ClusterFuzz has detected this issue as fixed in range 568671:568677.

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

Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x120356623ac0
Crash State:
  blink::AXObjectCacheImpl::GetOrCreate
  blink::AXObjectCacheImpl::HandleUpdateActiveMenuOption
  blink::LayoutMenuList::UpdateFromElement
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=563343:563345
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=568671:568677

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

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz has detected this issue as fixed in range 568671:568676.

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

Fuzzer: inferno_twister
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x127dd4db28c0
Crash State:
  blink::AXObjectCacheImpl::GetOrCreate
  blink::AXObjectCacheImpl::HandleUpdateActiveMenuOption
  blink::LayoutMenuList::UpdateFromElement
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=563343:563345
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=568671:568676

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

See https://github.com/google/clusterfuzz-tools 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 5734354119294976 is verified as fixed, so closing issue as verified.

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

Thanks for fixing this! Looks like clusterfuzz liked your fix. does it need a merge?

@wfh, it turns out the bug goes back pretty far, and the regression range is wrong.

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