Bug 159330

Summary: Possible null Range dereference under AXObjectCache::visiblePositionFromCharacterOffset()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: AccessibilityAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, enrica, jcraig, jdiggs, mario, n_wang, rniwa, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 158138    
Attachments:
Description Flags
Patch none

Description Chris Dumez 2016-06-30 21:30:16 PDT
Possible null Range dereference under AXObjectCache::visiblePositionFromCharacterOffset():
Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000018
Exception Note:        EXC_CORPSE_NOTIFY

VM Regions Near 0x18:
--> 
    __TEXT                 0000000107708000-000000010770a000 [    8K] r-x/rwx SM=COW  /System/Library/Frameworks/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.xpc/Contents/MacOS/com.apple.WebKit.WebContent

Application Specific Information:
Bundle controller class:
BrowserBundleController
Thread 0 Crashed ↩:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00007fffa724d981 WebCore::Range::startPosition() const + 17
1   com.apple.WebCore             	0x00007fffa7385695 WebCore::AXObjectCache::visiblePositionFromCharacterOffset(WebCore::CharacterOffset const&) + 53
2   com.apple.WebCore             	0x00007fffa738553c WebCore::AXObjectCache::setTextMarkerDataWithCharacterOffset(WebCore::TextMarkerData&, WebCore::CharacterOffset const&) + 140
3   com.apple.WebCore             	0x00007fffa7385f63 WebCore::AXObjectCache::startOrEndTextMarkerDataForRange(WebCore::TextMarkerData&, WTF::RefPtr<WebCore::Range>, bool) + 147
4   com.apple.WebCore             	0x00007fffa810f1e0 startOrEndTextmarkerForRange(WebCore::AXObjectCache*, WTF::RefPtr<WebCore::Range>, bool) + 48
5   com.apple.WebCore             	0x00007fffa810efca -[WebAccessibilityObjectWrapper textMarkerRangeFromRange:] + 154
6   com.apple.WebCore             	0x00007fffa811f5d2 -[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:] + 5794
7   com.apple.AppKit              	0x00007fff9ead94cb ___NSAccessibilityEntryPointValueForAttributeWithParameter_block_invoke.824 + 416
Comment 1 Radar WebKit Bug Importer 2016-06-30 21:30:45 PDT
<rdar://problem/27123752>
Comment 2 Chris Dumez 2016-06-30 21:34:14 PDT
Created attachment 282511 [details]
Patch
Comment 3 Benjamin Poulain 2016-06-30 23:21:45 PDT
Comment on attachment 282511 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282511&action=review

Can you please try to write a test before landing?

> Source/WebCore/accessibility/AXObjectCache.cpp:1961
> +    auto range = rangeForUnorderedCharacterOffsets(characterOffset, characterOffset);

Honestly, this auto is making this code worse.
I would prefer if you used the type.
Comment 4 Chris Dumez 2016-07-01 14:14:51 PDT
+ cfleizach / n_wang in case they know how to write a test for this as I have no idea.
Comment 5 chris fleizach 2016-07-01 15:09:27 PDT
(In reply to comment #4)
> + cfleizach / n_wang in case they know how to write a test for this as I
> have no idea.

There are some existing text marker range tests. My guess is you could make some invalid text marker ranges and pass into the api that calls into this method to recreate
Comment 6 Chris Dumez 2016-07-01 16:06:27 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > + cfleizach / n_wang in case they know how to write a test for this as I
> > have no idea.
> 
> There are some existing text marker range tests. My guess is you could make
> some invalid text marker ranges and pass into the api that calls into this
> method to recreate

OK, it looks like I have a test, thanks.
Comment 7 Chris Dumez 2016-07-01 16:11:47 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > + cfleizach / n_wang in case they know how to write a test for this as I
> > > have no idea.
> > 
> > There are some existing text marker range tests. My guess is you could make
> > some invalid text marker ranges and pass into the api that calls into this
> > method to recreate
> 
> OK, it looks like I have a test, thanks.

Actually no, the crash was in WKTR code.
Comment 8 Chris Dumez 2016-07-01 16:25:31 PDT
Comment on attachment 282511 [details]
Patch

Clearing flags on attachment: 282511

Committed r202762: <http://trac.webkit.org/changeset/202762>
Comment 9 Chris Dumez 2016-07-01 16:25:37 PDT
All reviewed patches have been landed.  Closing bug.