Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sensitive local storage data handling #28

Open
marzavec opened this issue Dec 15, 2020 · 7 comments
Open

Sensitive local storage data handling #28

marzavec opened this issue Dec 15, 2020 · 7 comments
Labels
enhancement New feature or request low priority Nbd, chill out

Comments

@marzavec
Copy link
Member

In issue #19 @henrywright raised concerns over storing the client password in local storage on public machines. The tl;dr is that he suggests clearing local storage once the app is closed or placing a button for specifically clearing it.

Personally, I don't feel like this is a big issue because of how hack.chat works. On other frameworks, someone logging into your computer and using your password/account means that they have access to all your account data (messages, friends, etc). On hack.chat, someone logging into your computer and using your password means. . . Not much. They can impersonate you, I suppose, if they know the channels you use.

The biggest issue would be password reuse. If the password you use on hack.chat is the same password you use for your gmail, then sure, that's a problem. . . Also; don't reuse passwords, anywhere.

To summarize; I feel that this is a knee-jerk reaction to the term "password", hence why I'm labeling it a low-priority feature request.

The data could be automatically deleted when the user closes hack.chat or alternatively a button could be provided for the user to click.

The first suggest would invalidate the use of local storage. The second is idea may be something we could place under the settings page, to clear all local storage?

To address the issue directly though, what if we placed a checkbox on the join pop up labeled "remember me"? If checked (by default), the password will be saved.

@marzavec marzavec added enhancement New feature or request low priority Nbd, chill out labels Dec 15, 2020
@henrywright
Copy link

The "reminder me" idea is definitely an improvement but it doesn't fully solve the problem. In the scenario you mention where a user has the same password for multiple services that password will still be stored in plain text in a publicly accessible place in the case where the computer is shared

@marzavec
Copy link
Member Author

same password for multiple services that password will still be stored in plain text

This is true, now the attacker only needs to guess your email. . .

it doesn't fully solve the problem

How so? If unchecked, the password will not be placed into local storage.

@henrywright
Copy link

How so?

The "remember me" option we are discussing if selected will store the password in local storage. The problem will still exist for the set of users opting in

@marzavec
Copy link
Member Author

Sure, so we are trying to find a solution for the set of users who are on public computers, using the "remember my password" option, who also happen to be using the same password as their important accounts- the main concern being that an attacker will pull up their local storage data, find the password and use it to login to said important accounts, because they have the email or username for whatever platform matches the password they find.

Sounds like I may have to reconsider the low-priority label.

The password could be hashed before being placed into local storage- however this comes with it's own complications, effects entropy and would be ultimately futile; any salting routine would be boil down to being static and the attacker just has to brute force the hash in order to gain their password.

Did you have a new suggestion?

@henrywright
Copy link

I get why this is an edge case but when a user submits a password they expect it to be protected. In the tiny number of scenarios you mentioned this just isn't the case

Hashing is a great idea. Brutally forceful attempts could reveal the password I agree but it is yet another hoop an attacker will need to jump through

@marzavec
Copy link
Member Author

when a user submits a password

That's an easy fix then; what if we change the terminology from "password" to "trip phrase"? Technically it isn't a password, as there are no accounts. . . :D

@henrywright
Copy link

In order to join a channel we just need the user to generate a trip to "sign" the messages they post. Using "trip phrase" terminology instead of "password" would solve the password being used elsewhere problem. This gets my vote

Of course an attacker could still use a "trip phrase" to impersonate a user or even perform actions if the trip phrase relates to a moderator or an administrator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority Nbd, chill out
Projects
None yet
Development

No branches or pull requests

2 participants