[SECURITY] Fix for credential leak vector #230

browserpass / browserpass-legacy

Legacy Browserpass repo, development is now happening at:

https://github.com/browserpass/browserpass-extension

MIT License

1k stars 80 forks source link

[SECURITY] Fix for credential leak vector #230 #231

Closed erayd closed 6 years ago

erayd commented 6 years ago

This PR fixes #230:

Credentials are now discarded immediately as soon as the tab has loaded
Credentials are not supplied to any model login that occurs after the tab has loaded
If the domain requesting credentials is not the same as the domain that was launched, the user will be asked if they really want this.

Once again, I apologise for #230 - that is my fault, and my oversight. I'm usually more careful than that, and I sincerely regret that I allowed this one to slip through.

erayd commented 6 years ago

@maximbaz Any chance of a code review? I'd rather this was merged sooner than later to get that hole closed off ASAP.

maximbaz commented 6 years ago

Credentials are now discarded immediately as soon as the tab has loaded

So now we intentionally make it so that browserpass will never fill credentials on https://www.httpwatch.com/httpgallery/authentication/ after clicking on "Display image", even if this page was opened via browserpass. Is that right?

erayd commented 6 years ago

So now we intentionally make it so that browserpass will never fill credentials on https://www.httpwatch.com/httpgallery/authentication/ after clicking on "Display image", even if this page was opened via browserpass. Is that right?

Yes.

Going forward, I'd like to address this use-case by intercepting the auth request and allowing the user to choose a credential, but I need to think about how to do this in a sane manner. In the meantime, simply disallowing the scenario feels much safer.