Bug 254798

Summary: AXObjectCache::characterOffsetFromVisiblePosition can deref a nullptr when underlying renderer is destroyed
Product: WebKit Reporter: Tyler Wilcock <tyler_w>
Component: AccessibilityAssignee: Tyler Wilcock <tyler_w>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Tyler Wilcock
Reported 2023-03-30 23:41:56 PDT
AXObjectCache::characterOffsetFromVisiblePosition creates an AX object from the node backing a VisiblePosition at the beginning of the method. Then, it does non-trivial work that could cause the renderer backing the AX object to be destroyed, and afterwards unconditionally deferences that AX object's node() at the end of the method. This can cause a null pointer dereference crash (because AccessibilityRenderObject::node() depends on a non-null renderer), and is generally poor pointer hygiene.
Attachments
Patch (3.19 KB, patch)
2023-03-30 23:47 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2023-03-30 23:42:08 PDT
<rdar://problem/107459184>
Tyler Wilcock
Comment 2 2023-03-30 23:42:54 PDT
rdar://103456792
Tyler Wilcock
Comment 3 2023-03-30 23:47:09 PDT
Created attachment 465696 [details] Patch
chris fleizach
Comment 4 2023-03-31 10:46:43 PDT
Do we still want to check that this is NOT null first? deepPos.deprecatedNode();
Tyler Wilcock
Comment 5 2023-03-31 10:56:30 PDT
(In reply to chris fleizach from comment #4) > Do we still want to check that this is NOT null first? > > deepPos.deprecatedNode(); We should be safe because if `deepPos.deprecatedNode()` were null, this check just above dereferencing it would return: if (visiblePos.isNull()) return CharacterOffset();
EWS
Comment 6 2023-03-31 11:58:15 PDT
Committed 262432@main (7d93b07962d5): <https://commits.webkit.org/262432@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 465696 [details].
Note You need to log in before you can comment on or make changes to this bug.