113377 2013-03-27 02:06:22 -0700 [Chromium] REGRESSION(r88030): Right-click on invalid form controls unexpectedly dispatches 'invalid' events 2013-03-28 14:45:13 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 tkent tkent dglazkov webkit.review.bot oldest_to_newest 864317 0 tkent 2013-03-27 02:06:22 -0700 https://code.google.com/p/chromium/issues/detail?id=126386 (Note: this report doesn't mention possibility of crashes.) http://trac.webkit.org/changeset/88030 In the preparation code of context menu, HTMLFormElement* form = selectedFrame->selection()->currentForm(); if (form && form->checkValidity() && r.innerNonSharedNode()->hasTagName(HTMLNames::inputTag)) { HTMLInputElement* selectedElement = static_cast<HTMLInputElement*>(r.innerNonSharedNode()); if (selectedElement) { WebSearchableFormData ws = WebSearchableFormData(WebFormElement(form), WebInputElement(selectedElement)); form->checkValidity() can dispatch 'invalid' events, and the 'form' object can be removed in event handlers. 864333 1 195252 tkent 2013-03-27 02:46:52 -0700 Created attachment 195252 Patch 864334 2 195252 tkent 2013-03-27 02:48:43 -0700 Comment on attachment 195252 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195252&action=review > Source/WebKit/chromium/src/ContextMenuClientImpl.cpp:341 > - if (form && form->checkValidity() && r.innerNonSharedNode()->hasTagName(HTMLNames::inputTag)) { > + if (form && r.innerNonSharedNode()->hasTagName(HTMLNames::inputTag)) { I think we have no reason to call checkValidity here, and functions dispatching events should not be called during context menu preparation. 865154 3 tkent 2013-03-27 21:40:42 -0700 Ah, I found this didn't cause use-after-free. If form->checkValidity() dispatches 'invalid' events, it returns false. So 'form' won't be accessed in such case. If form->checkValidity() returns true, it won't dispatch events and accessing 'form' is safe. 865816 4 195252 webkit.review.bot 2013-03-28 14:45:10 -0700 Comment on attachment 195252 Patch Clearing flags on attachment: 195252 Committed r147161: <http://trac.webkit.org/changeset/147161> 865817 5 webkit.review.bot 2013-03-28 14:45:13 -0700 All reviewed patches have been landed. Closing bug. 195252 2013-03-27 02:46:52 -0700 2013-03-28 14:45:09 -0700 Patch bug-113377-20130327184234.patch text/plain 1821 tkent U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ2OTUxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCmluZGV4 IDdkODcyYjVkN2Y1OTcwNzA2MTU2Yzk0Y2FjYzlkNzRhNjRlYmZjMzUuLjEwNGNlOTUzNWVkZDM5 NWVjOWEyNjY2ODRhOTFjZWRiZDRkOWYwZTEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTQgQEAKKzIwMTMtMDMtMjcgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0u b3JnPgorCisgICAgICAgIFtDaHJvbWl1bV0gUkVHUkVTU0lPTihyODgwMzApOiBSaWdodC1jbGlj ayBvbiBpbnZhbGlkIGZvcm0gY29udHJvbHMgdW5leHBlY3RlZGx5IGRpc3BhdGNoZXMgJ2ludmFs aWQnIGV2ZW50cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTEzMzc3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgKiBzcmMvQ29udGV4dE1lbnVDbGllbnRJbXBsLmNwcDoKKyAgICAgICAgKFdlYktpdDo6Q29u dGV4dE1lbnVDbGllbnRJbXBsOjpnZXRDdXN0b21NZW51RnJvbURlZmF1bHRJdGVtcyk6CisgICAg ICAgIFdlIGRvbid0IG5lZWQgdG8gY2FsbCBIVE1MRm9ybUVsZW1lbnQ6OmNoZWNrVmFsaWRpdHkg aGVyZS4KKwogMjAxMy0wMy0yNiAgUnlvc3VrZSBOaXdhICA8cm5pd2FAd2Via2l0Lm9yZz4KIAog ICAgICAgICBIZWFwLXVzZS1hZnRlci1mcmVlIHJlZ3Jlc3Npb24KZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJLaXQvY2hyb21pdW0vc3JjL0NvbnRleHRNZW51Q2xpZW50SW1wbC5jcHAgYi9Tb3VyY2Uv V2ViS2l0L2Nocm9taXVtL3NyYy9Db250ZXh0TWVudUNsaWVudEltcGwuY3BwCmluZGV4IGU3Nzc2 NGFmZTVkMTlmNjdmNjIxNmVlZDM4ZGFmODA4MzVlYTliOGMuLjBjY2ViODFhMTk5YWRmODZjOTU3 NTkzMGEzMDZlODgwZWUzNjAwYTAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0v c3JjL0NvbnRleHRNZW51Q2xpZW50SW1wbC5jcHAKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1 bS9zcmMvQ29udGV4dE1lbnVDbGllbnRJbXBsLmNwcApAQCAtMzM4LDcgKzMzOCw3IEBAIFBsYXRm b3JtTWVudURlc2NyaXB0aW9uIENvbnRleHRNZW51Q2xpZW50SW1wbDo6Z2V0Q3VzdG9tTWVudUZy b21EZWZhdWx0SXRlbXMoCiAgICAgICAgICAgICB9CiAgICAgICAgIH0KICAgICAgICAgSFRNTEZv cm1FbGVtZW50KiBmb3JtID0gc2VsZWN0ZWRGcmFtZS0+c2VsZWN0aW9uKCktPmN1cnJlbnRGb3Jt KCk7Ci0gICAgICAgIGlmIChmb3JtICYmIGZvcm0tPmNoZWNrVmFsaWRpdHkoKSAmJiByLmlubmVy Tm9uU2hhcmVkTm9kZSgpLT5oYXNUYWdOYW1lKEhUTUxOYW1lczo6aW5wdXRUYWcpKSB7CisgICAg ICAgIGlmIChmb3JtICYmIHIuaW5uZXJOb25TaGFyZWROb2RlKCktPmhhc1RhZ05hbWUoSFRNTE5h bWVzOjppbnB1dFRhZykpIHsKICAgICAgICAgICAgIEhUTUxJbnB1dEVsZW1lbnQqIHNlbGVjdGVk RWxlbWVudCA9IHN0YXRpY19jYXN0PEhUTUxJbnB1dEVsZW1lbnQqPihyLmlubmVyTm9uU2hhcmVk Tm9kZSgpKTsKICAgICAgICAgICAgIGlmIChzZWxlY3RlZEVsZW1lbnQpIHsKICAgICAgICAgICAg ICAgICBXZWJTZWFyY2hhYmxlRm9ybURhdGEgd3MgPSBXZWJTZWFyY2hhYmxlRm9ybURhdGEoV2Vi Rm9ybUVsZW1lbnQoZm9ybSksIFdlYklucHV0RWxlbWVudChzZWxlY3RlZEVsZW1lbnQpKTsK