Bug 43040 – cross_fuzz WebCore::SelectionController::isFocusedAndActive ReadAV@NULL (9e865de49b1800ec790dcc35d8ebd069)

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Berend-Jan Wever]

Created attachment 62679 [details]
Details

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

http://trac.webkit.org/browser/trunk/WebCore/css/CSSStyleSelector.cpp#L2510
2073 bool CSSStyleSelector::SelectorChecker::checkOneSelector(CSSSelector* sel, Element* e,
         HashSet<AtomicStringImpl*>* selectorAttrs, PseudoId& dynamicPseudo, bool isSubSelector,
         RenderStyle* elementStyle, RenderStyle* elementParentStyle) const
2074 {
<snip>
2186        // Normal element pseudo class checking.
2187         switch (sel->pseudoType()) {
<snip>
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.

http://trac.webkit.org/browser/trunk/WebCore/editing/SelectionController.cpp#L1403
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.

[Adam Barth]

frame()->selection() can't be null.  It's a component object of the frame.  The frame itself must be null.

[Adam Barth]

Created attachment 63831 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 63831 [details]
Patch

LGTM.

[Eric Seidel (no email)]

Comment on attachment 63831 [details]
Patch

Clearing flags on attachment: 63831

Committed r64947: <http://trac.webkit.org/changeset/64947>

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Is there a reason why this didn't have a regression test?

[Adam Barth]

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