109287 2013-02-08 05:07:28 -0800 [GTK] Crash in webkitURIResponseSetCertificateInfo() 2013-02-19 09:51:28 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED http://renevier.net/misc/webkit_109225.php P2 Normal --- 109566 110190 0 csaavedra webkit-unassigned a.renevier buildbot cgarcia gustavo mrobinson rniwa webkit.review.bot oldest_to_newest 828456 0 csaavedra 2013-02-08 05:07:28 -0800 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. 828458 1 187294 csaavedra 2013-02-08 05:19:27 -0800 Created attachment 187294 Patch 828463 2 webkit.review.bot 2013-02-08 05:23:26 -0800 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 828494 3 187294 buildbot 2013-02-08 06:00:46 -0800 Comment on attachment 187294 Patch Attachment 187294 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 828562 4 svillar 2013-02-08 07:40:51 -0800 *** Bug 109225 has been marked as a duplicate of this bug. *** 828637 5 a.renevier 2013-02-08 09:38:08 -0800 (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. 828997 6 cgarcia 2013-02-09 03:21:12 -0800 (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. 829056 7 a.renevier 2013-02-09 10:09:52 -0800 (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. 829057 8 a.renevier 2013-02-09 10:11:30 -0800 (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. 829059 9 187436 a.renevier 2013-02-09 10:16:13 -0800 Created attachment 187436 test page source 830787 10 187294 cgarcia 2013-02-12 04:54:21 -0800 Comment on attachment 187294 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 836496 11 cgarcia 2013-02-19 09:51:28 -0800 This is fixed now since webkitURIResponseSetCertificateInfo() doesn't exist anymore. 187294 2013-02-08 05:19:27 -0800 2013-02-12 04:54:21 -0800 Patch bug-109287-20130208151600.patch text/plain 1835 csaavedra U3VidmVyc2lvbiBSZXZpc2lvbjogMTQyMDk1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggOTU5OGU1MDIyMzU3Y2E0 ZDk3MjhjYjdlMTU4ZmIwYmJiYmMxODY0Yy4uYWI0MmYzMjkyMDEzYWZmOWI5NDk1OWIzODE0ZWM3 MTkzYTFkNzI4MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEzLTAyLTA4ICBDbGF1 ZGlvIFNhYXZlZHJhICA8Y3NhYXZlZHJhQGlnYWxpYS5jb20+CisKKyAgICAgICAgW0dUS10gQ3Jh c2ggaW4gd2Via2l0VVJJUmVzcG9uc2VTZXRDZXJ0aWZpY2F0ZUluZm8oKQorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA5Mjg3CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvQVBJL2d0ay9X ZWJLaXRXZWJWaWV3LmNwcDoKKyAgICAgICAgKHNldENlcnRpZmljYXRlVG9NYWluUmVzb3VyY2Up OiBEbyBub3QgYXNzdW1lIHRoZSBtYWluIHJlc291cmNlCisgICAgICAgIGFscmVhZHkgaGFzIGEg dXJpIHJlc3BvbnNlLgorCiAyMDEzLTAxLTMxICBDbGF1ZGlvIFNhYXZlZHJhICA8Y3NhYXZlZHJh QGlnYWxpYS5jb20+CiAKICAgICAgICAgW1dLMl1bTm90aWZpY2F0aW9uc10gTWlzc2luZyBlYXJs eSByZXR1cm4gaW4gcG9wdWxhdGVDb3B5T2ZOb3RpZmljYXRpb25QZXJtaXNzaW9ucwpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlldy5jcHAg Yi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRXZWJWaWV3LmNwcAppbmRl eCBlN2E3NTAxNWIyMmYxNTI0NmY0YWMwNjZjMWM1ZjFjZjZmNmZlYWI0Li44YjMxMTMxYTYzNDYw ZWU3YjhmNTU3MTI3ZjQxZGU3M2U2YzI5Y2UzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9V SVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRXZWJWaWV3LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9V SVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRXZWJWaWV3LmNwcApAQCAtMTMxNSw4ICsxMzE1LDggQEAg c3RhdGljIHZvaWQgc2V0Q2VydGlmaWNhdGVUb01haW5SZXNvdXJjZShXZWJLaXRXZWJWaWV3KiB3 ZWJWaWV3KQogICAgIFdlYktpdFdlYlZpZXdQcml2YXRlKiBwcml2ID0gd2ViVmlldy0+cHJpdjsK ICAgICBBU1NFUlQocHJpdi0+bWFpblJlc291cmNlLmdldCgpKTsKIAotICAgIHdlYmtpdFVSSVJl c3BvbnNlU2V0Q2VydGlmaWNhdGVJbmZvKHdlYmtpdF93ZWJfcmVzb3VyY2VfZ2V0X3Jlc3BvbnNl KHByaXYtPm1haW5SZXNvdXJjZS5nZXQoKSksCi0gICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgd2Via2l0V2ViUmVzb3VyY2VHZXRGcmFtZShwcml2LT5tYWluUmVzb3VyY2Uu Z2V0KCkpLT5jZXJ0aWZpY2F0ZUluZm8oKSk7CisgICAgaWYgKFdlYktpdFVSSVJlc3BvbnNlKiBy ZXNwb25zZSA9IHdlYmtpdF93ZWJfcmVzb3VyY2VfZ2V0X3Jlc3BvbnNlKHByaXYtPm1haW5SZXNv dXJjZS5nZXQoKSkpCisgICAgICAgIHdlYmtpdFVSSVJlc3BvbnNlU2V0Q2VydGlmaWNhdGVJbmZv KHJlc3BvbnNlLCB3ZWJraXRXZWJSZXNvdXJjZUdldEZyYW1lKHByaXYtPm1haW5SZXNvdXJjZS5n ZXQoKSktPmNlcnRpZmljYXRlSW5mbygpKTsKIH0KIAogc3RhdGljIHZvaWQgd2Via2l0V2ViVmll d0VtaXRMb2FkQ2hhbmdlZChXZWJLaXRXZWJWaWV3KiB3ZWJWaWV3LCBXZWJLaXRMb2FkRXZlbnQg bG9hZEV2ZW50KQo= 187436 2013-02-09 10:16:13 -0800 2013-02-09 10:16:13 -0800 test page source webkit_109225.php application/x-php 245 a.renevier PD9waHAKJGNvbnRlbnQgPSAiPGh0bWw+aGVsbG8gd29ybGQ8L2h0bWw+IjsKJG1kNSA9IG1kNSgk Y29udGVudCk7CmlmICgkX1NFUlZFUlsiSFRUUF9JRl9OT05FX01BVENIIl0gPT0gJG1kNSkgewog ICAgaGVhZGVyKCRfU0VSVkVSWydTRVJWRVJfUFJPVE9DT0wnXS4nIDMwNCBOb3QgTW9kaWZpZWQn LCB0cnVlLCAzMDQpOwp9IGVsc2UgewogICAgaGVhZGVyKCJFVGFnOiAkbWQ1Iik7CiAgICBwcmlu dCAkY29udGVudDsKfQo/Pgo=