Possible null Range dereference under AXObjectCache::visiblePositionFromCharacterOffset()

[Chris Dumez]

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

[Radar WebKit Bug Importer]

<rdar://problem/27123752>

[Chris Dumez]

Created attachment 282511 [details]
Patch

[Benjamin Poulain]

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.

[Chris Dumez]

+ cfleizach / n_wang in case they know how to write a test for this as I have no idea.

[chris fleizach]

(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

[Chris Dumez]

(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.

[Chris Dumez]

(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.

[Chris Dumez]

Comment on attachment 282511 [details]
Patch

Clearing flags on attachment: 282511

Committed r202762: <http://trac.webkit.org/changeset/202762>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.