Bug 113377 - [Chromium] REGRESSION(r88030): Right-click on invalid form controls unexpectedly dispatches 'invalid' events
Summary: [Chromium] REGRESSION(r88030): Right-click on invalid form controls unexpecte...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-27 02:06 PDT by Kent Tamura
Modified: 2013-03-28 14:45 PDT (History)
2 users (show)

See Also:

Attachments
Patch (1.78 KB, patch)
2013-03-27 02:46 PDT, Kent Tamura 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2013-03-27 02:06:22 PDT 
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.
Comment 1 Kent Tamura 2013-03-27 02:46:52 PDT 
Created attachment 195252 [details]
Patch
Comment 2 Kent Tamura 2013-03-27 02:48:43 PDT 
Comment on attachment 195252 [details]
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.
Comment 3 Kent Tamura 2013-03-27 21:40:42 PDT 
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.
Comment 4 WebKit Review Bot 2013-03-28 14:45:10 PDT 
Comment on attachment 195252 [details]
Patch

Clearing flags on attachment: 195252

Committed r147161: <http://trac.webkit.org/changeset/147161>
Comment 5 WebKit Review Bot 2013-03-28 14:45:13 PDT 
All reviewed patches have been landed.  Closing bug.