New issue
Advanced search Search tips

Issue 810146 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security
Team-Accessibility



Sign in to add a comment

Heap-use-after-free in blink::LayoutObject::WillBeDestroyed

Project Member Reported by ClusterFuzz, Feb 7 2018

Issue description

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

Fuzzer: inferno_twister
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x12c612ebc428
Crash State:
  blink::LayoutObject::WillBeDestroyed
  blink::LayoutBoxModelObject::WillBeDestroyed
  blink::LayoutBox::WillBeDestroyed
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=531299:531319

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Feb 7 2018

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

Comment 2 by ClusterFuzz, Feb 8 2018

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

Comment 3 by ClusterFuzz, Feb 8 2018

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

Job Type: mac_asan_content_shell
Platform Id: mac

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x6130005007e8
Crash State:
  blink::LayoutObject::WillBeDestroyed
  blink::LayoutBoxModelObject::WillBeDestroyed
  blink::LayoutBox::WillBeDestroyed
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

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

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

Comment 4 by sheriffbot@chromium.org, Feb 8 2018

Labels: M-65
Project Member

Comment 5 by sheriffbot@chromium.org, Feb 8 2018

Labels: ReleaseBlock-Stable
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
Project Member

Comment 6 by sheriffbot@chromium.org, Feb 8 2018

Labels: Pri-1
Components: Blink>DOM>ShadowDOM
Owner: hayato@chromium.org
Status: Assigned (was: Untriaged)
c#3 has correct regression range. Looks like regression from
https://chromium.googlesource.com/chromium/src/+/53609b0173f42b54fe57ed50f9f6707adcb09c1c. Hayato@, can you please take a look.
Project Member

Comment 8 by sheriffbot@chromium.org, Feb 8 2018

Labels: -Security_Impact-Head Security_Impact-Beta
Owner: infe...@chromium.org
https://chromium.googlesource.com/chromium/src/+/53609b0173f42b54fe57ed50f9f6707adcb09c1c could not be a reason because it has no-effect unless IncrementalShadowDOM flag is enabled.

It's experimental flag and disabled by default.

inferno@, could you find another suspicious cl in the range?
Project Member

Comment 10 by ClusterFuzz, Feb 9 2018

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

Comment 11 by ClusterFuzz, Feb 9 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=6412861203808256.
Components: -Blink>DOM>ShadowDOM Blink>Accessibility
Owner: dmazz...@chromium.org
I see accessibility function in both crash and free stacks (and since this is only reproducing in content_shell on windows/mac). Dominicc, can you please take a look.
M65 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge  into the release branch ASAP. Thank you.
Labels: -Security_Impact-Beta -ReleaseBlock-Stable -M-65 Security_Impact-Head M-66
Project Member

Comment 15 by sheriffbot@chromium.org, Feb 17 2018

Labels: ReleaseBlock-Stable
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
Project Member

Comment 16 by bugdroid1@chromium.org, Feb 21 2018

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

commit eebef6ebc57245087743bcdc5b99dfa106843109
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Wed Feb 21 00:59:24 2018

Avoid creating AXObject at unsafe times

When a Node is deleted, we need to walk the accessibility tree to
determine events to fire, but we shouldn't create any new AXObjects
at that time because the DOM may be untrustworthy, leading to a crash.

We already had ComputeParentIfExists to handle that case, but it was
calling MenuButtonForMenu(), which was sometimes creating a node. Add
MenuButtonForMenuIfExists() for that important safe case.

Bug:  810146 
Change-Id: I27c5df196ff6f39dcefb5a353d73d75aa7a8f677
Reviewed-on: https://chromium-review.googlesource.com/927486
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537961}
[modify] https://crrev.com/eebef6ebc57245087743bcdc5b99dfa106843109/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp
[modify] https://crrev.com/eebef6ebc57245087743bcdc5b99dfa106843109/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
[modify] https://crrev.com/eebef6ebc57245087743bcdc5b99dfa106843109/third_party/WebKit/Source/modules/accessibility/AXNodeObject.h

Status: Fixed (was: Assigned)
Project Member

Comment 18 by ClusterFuzz, Feb 21 2018

ClusterFuzz has detected this issue as fixed in range 537954:537972.

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

Fuzzer: inferno_twister
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x12c612ebc428
Crash State:
  blink::LayoutObject::WillBeDestroyed
  blink::LayoutBoxModelObject::WillBeDestroyed
  blink::LayoutBox::WillBeDestroyed
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=531299:531319
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=537954:537972

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

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.
Project Member

Comment 19 by ClusterFuzz, Feb 21 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 4628679846789120 is verified as fixed, so closing issue as verified.

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

Comment 20 by sheriffbot@chromium.org, Feb 21 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 21 by sheriffbot@chromium.org, Mar 27 2018

Labels: -Security_Impact-Head Security_Impact-Beta
Labels: -ReleaseBlock-Stable
Project Member

Comment 23 by sheriffbot@chromium.org, Apr 27 2018

Labels: Merge-Request-67
Project Member

Comment 24 by sheriffbot@chromium.org, Apr 27 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 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), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The fix in #16 landed in M66, so I'm not sure why SheriffBot added a merge request.

https://storage.googleapis.com/chromium-find-releases-static/eeb.html#eebef6ebc57245087743bcdc5b99dfa106843109
Cc: awhalley@chromium.org
+awhalley@ for M67 merge review. 
Labels: -Merge-Review-67
Change listed at #16 is already in M67. So removing "Merge-Review-67" label.
Project Member

Comment 28 by sheriffbot@chromium.org, May 30 2018

Labels: -Restrict-View-SecurityNotify allpublic
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

Sign in to add a comment