Bug 43040

Summary: cross_fuzz WebCore::SelectionController::isFocusedAndActive ReadAV@NULL (9e865de49b1800ec790dcc35d8ebd069)
Product: WebKit Reporter: Berend-Jan Wever <skylined>
Component: DOMAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: abarth, ap, eric, lcamtuf
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows Vista   
URL: http://code.google.com/p/chromium/issues/detail?id=50352
Bug Depends on:    
Bug Blocks: 42959    
Description Flags
Patch none

Description Berend-Jan Wever 2010-07-27 04:26:54 PDT
Created attachment 62679 [details]

From analyzing crash details found by the fuzzer described in bug 42959, I seem to have found the following problem:

2073 bool CSSStyleSelector::SelectorChecker::checkOneSelector(CSSSelector* sel, Element* e,
         HashSet<AtomicStringImpl*>* selectorAttrs, PseudoId& dynamicPseudo, bool isSubSelector,
         RenderStyle* elementStyle, RenderStyle* elementParentStyle) const
2074 {
2186        // Normal element pseudo class checking.
2187         switch (sel->pseudoType()) {
2509             case CSSSelector::PseudoFocus:
2510                 if (e && e->focused() && e->document()->frame()->selection()->isFocusedAndActive())
2511                    return true;
2512                break;

e->document()->frame()->selection() can be NULL, the code does not take this into consideration.

1402	bool SelectionController::isFocusedAndActive() const
1403	{
1404	    return m_focused && m_frame->page() && m_frame->page()->focusController()->isActive();
1405	}

Trying to read a property of a NULL object causes an access violation.

I've attached the details I extracted automatically using a debugger that helped me track down this issue.
Comment 1 Adam Barth 2010-08-07 14:40:14 PDT
frame()->selection() can't be null.  It's a component object of the frame.  The frame itself must be null.
Comment 2 Adam Barth 2010-08-07 14:44:32 PDT
Created attachment 63831 [details]
Comment 3 Eric Seidel (no email) 2010-08-07 18:18:50 PDT
Comment on attachment 63831 [details]

Comment 4 Eric Seidel (no email) 2010-08-08 01:07:29 PDT
Comment on attachment 63831 [details]

Clearing flags on attachment: 63831

Committed r64947: <http://trac.webkit.org/changeset/64947>
Comment 5 Eric Seidel (no email) 2010-08-08 01:07:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Alexey Proskuryakov 2010-08-09 01:10:27 PDT
Is there a reason why this didn't have a regression test?
Comment 7 Adam Barth 2010-08-09 10:45:12 PDT
> Is there a reason why this didn't have a regression test?

I'm not really sure how to call the API.  If we have infinite null checks on Frame* that don't have tests.  The code is wrong on its face.