New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 852735 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 16
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-08-20
OS: Linux , Mac
Pri: 1
Type: Bug
Team-Accessibility

Blocking:
issue 852251



Sign in to add a comment

Null-dereference WRITE in blink::AXObjectCacheImpl::PostNotification

Project Member Reported by ClusterFuzz, Jun 14 2018

Issue description

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

Fuzzer: mbarbella_webcomponents
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference WRITE
Crash Address: 0x0000000000a0
Crash State:
  blink::AXObjectCacheImpl::PostNotification
  blink::AXNodeObject::ChildrenChanged
  blink::AXObjectCacheImpl::ChildrenChanged
  
Sanitizer: address (ASAN)

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: brajkumar@chromium.org
Components: Blink>Accessibility
Labels: M-69 Test-Predator-Wrong CF-NeedsTriage
Unable to find actual suspect through code search and also observing no CL's under regression range, hence adding appropriate label and requesting someone from blink team to look in to this issue.

Thanks!
Labels: -OS-Linux ReleaseBlock-Stable
Owner: dmazz...@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 3 by ClusterFuzz, Jun 18 2018

Labels: OS-Linux
Project Member

Comment 4 by ClusterFuzz, Jun 22 2018

Labels: OS-Mac
Dominic, M69 already branched, please expedite the fix.
M69 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.
M69 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.


Better fix, just need to make a couple of tests pass:
https://chromium-review.googlesource.com/c/chromium/src/+/1175491

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 15

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

commit 38e4262e2a3c0e1d00cfb4db96efacb775c2b9aa
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Wed Aug 15 17:03:51 2018

Handle case where AXObject is destroyed in the middle of a method.

Bug:  852735 
Change-Id: I56752deea548f5aa01f9ae3d71f3551841737809
Reviewed-on: https://chromium-review.googlesource.com/1175489
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583285}
[modify] https://crrev.com/38e4262e2a3c0e1d00cfb4db96efacb775c2b9aa/third_party/blink/renderer/modules/accessibility/ax_node_object.cc

NextAction: 2018-08-16
Pls update bug with canary result tomorrow and request a merge to M69 once fix is ready to merge.
Project Member

Comment 12 by ClusterFuzz, Aug 16

ClusterFuzz has detected this issue as fixed in range 583284:583288.

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

Fuzzer: mbarbella_webcomponents
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference WRITE
Crash Address: 0x0000000000a0
Crash State:
  blink::AXObjectCacheImpl::PostNotification
  blink::AXNodeObject::ChildrenChanged
  blink::AXObjectCacheImpl::ChildrenChanged
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=566621:566623
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=583284:583288

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

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 13 by ClusterFuzz, Aug 16

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.
The NextAction date has arrived: 2018-08-16
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 16

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

commit 98acdea74f9d50d8ce3776443a8ba194907f6970
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Aug 16 17:59:36 2018

Avoid recalculating native role in AXNodeObject::CanHaveChildren

CanHaveChildren() is called frequently, including from delicate
situations like when a LayoutObject is being deleted. Currently it
calls NativeAccessibilityRoleIgnoringAria(), which is actually
somewhat expensive and calls UpdateDistribution(), and clusterfuzz
found a way for that to result in a UAF.

The easy solution is just to save the result of
NativeAccessibilityRoleIgnoringAria when we initially
compute the node's role. Then CanHaveChildren() can just
check it directly rather than recomputing it each time.

Bug:  852735 ,  852251 
Change-Id: Id745d3b42c1f89434e519195e8511159621734d0
Reviewed-on: https://chromium-review.googlesource.com/1175491
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583726}
[modify] https://crrev.com/98acdea74f9d50d8ce3776443a8ba194907f6970/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc
[modify] https://crrev.com/98acdea74f9d50d8ce3776443a8ba194907f6970/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
[modify] https://crrev.com/98acdea74f9d50d8ce3776443a8ba194907f6970/third_party/blink/renderer/modules/accessibility/ax_node_object.h

Cc: awhalley@chromium.org
Is this need a merge to M69? 
Labels: Merge-Request-69
Requested merge to M69.

Blocking: 852251
NextAction: 2018-08-17
Pls update bug with canary result tomorrow.

awhalley@ (Security TPM) for M69 merge review as blocking  bug 852251  is a security bug.
Labels: -Merge-TBD
The NextAction date has arrived: 2018-08-17
govind@ - good for 69, but worth holding off to Monday just to send a little more time in Canary
NextAction: 2018-08-20
Sure, pls update bug with canary result on Monday morning.
Project Member

Comment 25 by sheriffbot@chromium.org, Aug 17

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The NextAction date has arrived: 2018-08-20
govind@ - good for 69
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #27. Please merge ASAP. Thank you.
Project Member

Comment 29 by bugdroid1@chromium.org, Aug 20

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d3bb5da1eeae4947d925f3337b7a0667403135e

commit 2d3bb5da1eeae4947d925f3337b7a0667403135e
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Aug 20 21:26:49 2018

Merge to M69: Avoid recalculating native role in AXNodeObject::CanHaveChildren

CanHaveChildren() is called frequently, including from delicate
situations like when a LayoutObject is being deleted. Currently it
calls NativeAccessibilityRoleIgnoringAria(), which is actually
somewhat expensive and calls UpdateDistribution(), and clusterfuzz
found a way for that to result in a UAF.

The easy solution is just to save the result of
NativeAccessibilityRoleIgnoringAria when we initially
compute the node's role. Then CanHaveChildren() can just
check it directly rather than recomputing it each time.

Bug:  852735 ,  852251 
Change-Id: Id745d3b42c1f89434e519195e8511159621734d0
Reviewed-on: https://chromium-review.googlesource.com/1175491
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583726}(cherry picked from commit 98acdea74f9d50d8ce3776443a8ba194907f6970)
Reviewed-on: https://chromium-review.googlesource.com/1182204
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#724}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/2d3bb5da1eeae4947d925f3337b7a0667403135e/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc
[modify] https://crrev.com/2d3bb5da1eeae4947d925f3337b7a0667403135e/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
[modify] https://crrev.com/2d3bb5da1eeae4947d925f3337b7a0667403135e/third_party/blink/renderer/modules/accessibility/ax_node_object.h

Labels: -ReleaseBlock-Stable

Sign in to add a comment