[Webkit-unassigned] [Bug 90839] New: Secure input can be enabled permanently due to logic error

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=90839

           Summary: Secure input can be enabled permanently due to logic
                    error
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Macintosh
        OS/Version: All
            Status: UNCONFIRMED
          Severity: Normal
          Priority: P2
         Component: Event Handling
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: greg at smilesoftware.com


Please refer to - [WKView _updateSecureInputState] (rev. 122110).

Note that when DisableSecureEventInput() is called, the internal flag is cleared: _data->_inSecureInputState = NO.

However later in the method when EnableSecureEventInput() is called, the internal flag is untouched. Same for the later call to DisableSecureEventInput().

This could lead to a situation where secure event input is enabled, but _data->_inSecureInputState is NO. If this method is called again, it will not disable secure event input because _data->_inSecureInputState is NO and so the initial test fails and secure input remains enabled.

Secure input remaining enabled is a problem for apps such as TextExpander:

http://smilesoftware.com/TextExpander/secureinput.html

Here's info from Apple on how to handle secure input:

http://developer.apple.com/library/mac/#technotes/tn2150/

We have a number of reports that this problem is common with Chrome. It's not common with Safari, but perhaps Safari has a higher-level workaround.

I'm not a member of the WebKit project, and I'm afraid I don't have a reproducible case to trigger the logic problem I'm positing here, but I'm filing this bug in hopes that I can interest someone on the dev team or with commit privileges to consider making a fix.

Here's the code I suggest. The only changes are to update _data->_inSecureInputState after the calls to EnableSecureEventInput() and DisableSecureEventInput() later in this method:

- (void)_updateSecureInputState
{
    if (![[self window] isKeyWindow] || ![self _isFocused]) {
        if (_data->_inSecureInputState) {
            DisableSecureEventInput();
            _data->_inSecureInputState = NO;
        }
        return;
    }
    // WKView has a single input context for all editable areas (except for plug-ins).
    NSTextInputContext *context = [super inputContext];
    bool isInPasswordField = _data->_page->editorState().isInPasswordField;

    if (isInPasswordField) {
        if (!_data->_inSecureInputState) {
            EnableSecureEventInput();
            _data->_inSecureInputState = YES;
        }
        static NSArray *romanInputSources = [[NSArray alloc] initWithObjects:&NSAllRomanInputSourcesLocaleIdentifier count:1];
        [context setAllowedInputSourceLocales:romanInputSources];
    } else {
        if (_data->_inSecureInputState) {
            DisableSecureEventInput();
            _data->_inSecureInputState = NO;
        }
        [context setAllowedInputSourceLocales:nil];
    }
    _data->_inSecureInputState = isInPasswordField;
}

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.