Bug 43040 - cross_fuzz WebCore::SelectionController::isFocusedAndActive ReadAV@NULL (9e865de49b1800ec790dcc35d8ebd069)
Summary: cross_fuzz WebCore::SelectionController::isFocusedAndActive ReadAV@NULL (9e86...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows Vista
: P1 Normal
Assignee: Nobody
URL: http://code.google.com/p/chromium/iss...
Keywords:
Depends on:
Blocks: 42959
  Show dependency treegraph
 
Reported: 2010-07-27 04:26 PDT by Berend-Jan Wever
Modified: 2010-08-09 10:45 PDT (History)
4 users (show)

See Also:


Attachments
Details (287.67 KB, text/html)
2010-07-27 04:26 PDT, Berend-Jan Wever
no flags Details
Patch (1.57 KB, patch)
2010-08-07 14:44 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Berend-Jan Wever 2010-07-27 04:26:54 PDT
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.
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]
Patch
Comment 3 Eric Seidel (no email) 2010-08-07 18:18:50 PDT
Comment on attachment 63831 [details]
Patch

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

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.