simsalabim/sisyphus

Security of "password" inputs

Closed this issue · 6 comments

Shouldn't Sisyphus always ignore password inputs?

If the value of password fields is stored in LocalStorage then any other script on the page could grab this data, and hijack the user's credentials as they're filling out a form.

I think this could be fixed by changing line 179 from:

var fieldsToProtect = $( this ).find( ":input" ).not( ":submit" ).not( ":reset" ).not( ":button" ).not( ":file" );

To this, which excludes "password" inputs.

var fieldsToProtect = $( this ).find( ":input" ).not( ":submit" ).not( ":reset" ).not( ":button" ).not( ":file" ).not( ":password");

+1 for this security issue. But I think it has workaround like this (I have not test it yet)

sisyphusObject.setOptions( {
    excludeFields: $("input:password")
})

@brendanf I didn't quite get the origin of the issue. If user manually enters his password in password field, any other script on the page still can grab this data. Not even talking about autofilled passwords inputs if the user has stored his passwords in a browser.

@giver yes this should work

@simsalabim Here's the use case: the form with Sisyphus is an SSL page but the entire site is not. If you partially completed the form and navigated elsewhere then your password could be exposed via Local Storage. The SSL page would be "locked down" from external scripts whereas the majority of the site doesn't need to be.

I'm thinking always blocking passwords is a better default than always including passwords. There are few cases where I'd want the user to have any password field stored clientside in this way.

@giver Thanks, that's what I'm doing currently but I thought this should be a default.

I think this is very much worth discussing since people get all freaked out when sensitive data is stored (temporarily or otherwise).

Although the likelihood that someone's data will be stolen through LocalStorage is small, it should probably not be the default to save it.

@simsalabim makes a logical point, however a user might still get freaked out that their password fields were remembered.

Thanks guys for pointing my attention to it, fixed and available in v1.1