Bug 109287 – [GTK] Crash in webkitURIResponseSetCertificateInfo()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Claudio Saavedra]

Stacktrace:

#0  0x00007ffff6313647 in webkitURIResponseSetCertificateInfo(_WebKitURIResponse*, WebKit::WebCertificateInfo*) ()
   from /home/claudio/git/gnome/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#1  0x00007ffff6321dc1 in webkitWebViewLoadChanged(_WebKitWebView*, WebKitLoadEvent) ()
   from /home/claudio/git/gnome/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
#2  0x00007ffff63795e0 in WebKit::WebPageProxy:idCommitLoadForFrame(unsigned long, WTF:tring const&, bool, unsigned int, WebKit:latformCertificateInfo const&, CoreIPC::MessageDecoder&) ()

Quick analysis:

WebKitWebView's setCertificateToMainResource() is calling webkitURIResponseSetCertificateInfo() and passing an unchecked call to webkit_web_resource_get_response() as the WebKitURIResponse parameter. The docs for webkit_web_resource_get_response() tell that this function can return NULL but webkitURIResponseSetCertificateInfo() doesn't check for this and dereferences directly.

The quick fix would be not to call to webkitURIResponseSetCertificateInfo() if the webresource doesn't have yet a response, but I am not sure whether this is the right thing.

[Claudio Saavedra]

Created attachment 187294 [details]
Patch

[WebKit Review Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Build Bot]

Comment on attachment 187294 [details]
Patch

Attachment 187294 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16427616

New failing tests:
http/tests/cache/cached-main-resource.html

[Sergio Villar Senin]

*** Bug 109225 has been marked as a duplicate of this bug. ***

[arno.]

(In reply to comment #0)
> 
> The quick fix would be not to call to webkitURIResponseSetCertificateInfo() if the webresource doesn't have yet a response, but I am not sure whether this is the right thing.


Then, when response arrives later, it would not get a certificate info. What are the implications of that ?

Would that work better by always registering a notify::response callback on the main response, and calling setCertificateToMainResource from there ?


Also, even with that patch, there still is problem:
at *third* load of a page with 304 code, the page does not load (try to go to http://localhost/tmp/crash.php at 3rd load, network panel in error console show that the request has been aborted). That is because webkitWebViewDecidePolicy receives the 304 response with a null mimetype the second time (but not the first time). That is probably another bug though.

[Carlos Garcia Campos]

(In reply to comment #0)
> Stacktrace:
> 
> #0  0x00007ffff6313647 in webkitURIResponseSetCertificateInfo(_WebKitURIResponse*, WebKit::WebCertificateInfo*) ()
>    from /home/claudio/git/gnome/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
> #1  0x00007ffff6321dc1 in webkitWebViewLoadChanged(_WebKitWebView*, WebKitLoadEvent) ()
>    from /home/claudio/git/gnome/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
> #2  0x00007ffff63795e0 in WebKit::WebPageProxy:idCommitLoadForFrame(unsigned long, WTF:tring const&, bool, unsigned int, WebKit:latformCertificateInfo const&, CoreIPC::MessageDecoder&) ()
> 
> Quick analysis:
> 
> WebKitWebView's setCertificateToMainResource() is calling webkitURIResponseSetCertificateInfo() and passing an unchecked call to webkit_web_resource_get_response() as the WebKitURIResponse parameter. The docs for webkit_web_resource_get_response() tell that this function can return NULL but webkitURIResponseSetCertificateInfo() doesn't check for this and dereferences directly.

webkit_we_resource_get_response can return NULL, but it should never happen when setCertificateToMainResource is called, how can I reproduce this crash?

> The quick fix would be not to call to webkitURIResponseSetCertificateInfo() if the webresource doesn't have yet a response, but I am not sure whether this is the right thing.

[arno.]

(In reply to comment #6)
> (In reply to comment #0)
> > Stacktrace:
> > 
> > #0  0x00007ffff6313647 in webkitURIResponseSetCertificateInfo(_WebKitURIResponse*, WebKit::WebCertificateInfo*) ()
> >    from /home/claudio/git/gnome/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
> > #1  0x00007ffff6321dc1 in webkitWebViewLoadChanged(_WebKitWebView*, WebKitLoadEvent) ()
> >    from /home/claudio/git/gnome/WebKit/WebKitBuild/Release/.libs/libwebkit2gtk-3.0.so.0
> > #2  0x00007ffff63795e0 in WebKit::WebPageProxy:idCommitLoadForFrame(unsigned long, WTF:tring const&, bool, unsigned int, WebKit:latformCertificateInfo const&, CoreIPC::MessageDecoder&) ()
> > 
> > Quick analysis:
> > 
> > WebKitWebView's setCertificateToMainResource() is calling webkitURIResponseSetCertificateInfo() and passing an unchecked call to webkit_web_resource_get_response() as the WebKitURIResponse parameter. The docs for webkit_web_resource_get_response() tell that this function can return NULL but webkitURIResponseSetCertificateInfo() doesn't check for this and dereferences directly.
> 
> webkit_we_resource_get_response can return NULL, but it should never happen when setCertificateToMainResource is called, how can I reproduce this crash?
> 
> > The quick fix would be not to call to webkitURIResponseSetCertificateInfo() if the webresource doesn't have yet a response, but I am not sure whether this is the right thing.

[arno.]

(In reply to comment #7)

> > webkit_we_resource_get_response can return NULL, but it should never happen when setCertificateToMainResource is called, how can I reproduce this crash?

Oups, looks like I copy/pasted the wrong url.
the crash happens in case of a 304 http code.
If you load http://renevier.net/misc/webkit_109225.php in MiniBrowser and reload it, it should crash.

[arno.]

Created attachment 187436 [details]
test page source

[Carlos Garcia Campos]

Comment on attachment 187294 [details]
Patch

This is not the right fix, as I said, that situation should never happen and it's actually a bug in WebCore, see bug https://bugs.webkit.org/show_bug.cgi?id=109566

[Carlos Garcia Campos]

This is fixed now since webkitURIResponseSetCertificateInfo() doesn't exist anymore.