Bug 119231 – Null deref under PluginView::handlesPageScaleFactor()

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Tim Horton]

PluginView::handlesPageScaleFactor() doesn't null-check m_plugin, but it should. It should check isInitialized as well!

<rdar://problem/14440207>

[Tim Horton]

Created attachment 207681 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 207681 [details]
patch

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

> Source/WebKit2/WebProcess/Plugins/PluginView.h:90
> +    bool handlesPageScaleFactor();

Can this be const?

[Tim Horton]

(In reply to comment #2)
> (From update of attachment 207681 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207681&action=review
> 
> > Source/WebKit2/WebProcess/Plugins/PluginView.h:90
> > +    bool handlesPageScaleFactor();
> 
> Can this be const?

Yep. pageScaleFactor() too!

Thanks!

http://trac.webkit.org/changeset/153449

[Tim Horton]

Created attachment 207686 [details]
another potential null

[Tim Horton]

Reopened for one more patch (so many things can be null).

[Tim Horton]

Created attachment 207689 [details]
try to make ews go

[Darin Adler]

Comment on attachment 207689 [details]
try to make ews go

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

> Source/WebKit2/WebProcess/WebPage/WebFrame.cpp:490
>      PluginDocument* pluginDocument = static_cast<PluginDocument*>(m_coreFrame->document());
> -    PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget());
> +    if (PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget()))
> +        return pluginView->handlesPageScaleFactor();
>  
> -    return pluginView->handlesPageScaleFactor();
> +    return 0;

I prefer early return to nesting the main line code in the if. Or using &&.

    return pluginView && pluginView->handlesPageScaleFactor();

[Tim Horton]

Thanks, Darin. I went with &&.

http://trac.webkit.org/changeset/153486