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

Possible vulnerability in browser extension caused by browser.storage.local API implementation #485

Closed
subdavis opened this issue Jan 18, 2018 · 23 comments

Comments

5 participants
@subdavis
Copy link

commented Jan 18, 2018

First, some background.

This may be speculation. I don't know how BitWarden works and I don't have the patience to dig through the code and find whether or not this is truly how BitWarden works.

The Issue

BitWarden defaults to never forget. I found this out by reading https://www.reddit.com/r/Bitwarden/comments/7m2u12/how_can_i_prevent_my_master_password_from_being/ which points to https://www.reddit.com/r/Bitwarden/comments/7i7olr/does_bitwarden_save_the_master_password_locally/

I recently discovered https://subdavis.com/blog/jekyll/update/2017/01/02/ckp-security-flaw.html in CKP, the keepass extension. Please read that thoroughly - it's just good to know and I think it applies here.

I looked through my localStorage logs for bitwarden, for me located at /home/brandon/.config/google-chrome/Default/Local Extension Settings/nngceckbapebfimnlniiiahkandclblb

Check out 000003.log. I can see a key/val pair that looks like key."6peZkNbLZ92w5Hlw7Vdup0WyS7YG3oz2h7Z6d6jCuy8=". For anyone curious why I would post that, I don't use bitwarden and my account contains no entries.

Now I change the settings so that Forget is not "never". In 000003.log, I see

key�݉�0
             �����T(���

It looks like the code attempted to clear the value of key, but failed to actually remove the copy in the journal file.

The interesting bit is when I turn "never" back on again. key."6peZkNbLZ92w5Hlw7Vdup0WyS7YG3oz2h7Z6d6jCuy8=" is again found at the bottom of my file - now there are three copies: the original, the overwrite, and the replacement (identical to the original)

My question.

Is key."6peZkNbLZ92w5Hlw7Vdup0WyS7YG3oz2h7Z6d6jCuy8=" enough to crack open my BitWarden vault if I have access to the encrypted data on BitWarden servers?

Does this mean every single BitWarden user is mucking about with a master key on their disk? (If this seems like a logical jump, read this again.

If that is the case, this cannot be solved by simply changing the default - if anyone ever uses the "never" feature temporarily, they're Pwnd for life.

My humble apology.

I'm not sure if ANY of this works like I have a hunch it does. If I'm wrong, I'm seriously very sorry to have alarmed anyone.

@subdavis subdavis changed the title Possible Vulnerability in browser extension caused by defaulting to never forget. Possible vulnerability in browser extension caused by browser.storage.local API implementation Jan 18, 2018

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

I just checked this on my local windows machine at C:\Users\me\AppData\Local\Google\Chrome\User Data\Default\Local Extension Settings\nngceckbapebfimnlniiiahkandclblb\000004.log and I cannot find any trace of key. or "key". I am using a lock option which would have cleared the default key which was originally saved after first login.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

Also verified with Firefox on windows at %AppData%\Roaming\Mozilla\Firefox\Profiles\your_profile\browser-extension-data\{446900e4-71c2-419f-a6a7-df9c091e268b}\storage.js. No trace of the key found in that file.

@subdavis

This comment has been minimized.

Copy link
Author

commented Jan 18, 2018

Either

  • Uninstall, reinstall, and look again on chrome, or
  • Toggle your remember option to "Never", look, change it back, and look again.

I'm having difficulty finding implementation details for Chrome's local storage, but every extension I've ever installed begins with 000003.log. I have extensions that have started there and when each file fills, it rolls over. My AdGuard plugin has made it to 000237.log

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

Looks like it's there after toggling which is weird. I am not sure the format of this file, but I can definitely say that is wasn't there before which means the value is not saved forever. Maybe it's some kind of cache?

I toggled Firefox as well but am unable to find any kind of trace of the key there.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

All of the data there looks like values that have recently changed. I'll keep monitoring the file and see if I can figure out when it "purges".

@subdavis

This comment has been minimized.

Copy link
Author

commented Jan 18, 2018

It isn't saved forever - It's saved until you fill up the log file and it rolls over to another.

This is how the chrome storage API works - the keyvalue store is implemented with a journaled logfile - the most recent value gets returned when you call chrome.storage.local.get() but all others are physically present on disk until either you call chrome.storage.local.clear() or the file fills to capacity and rolls over. Depending on how often you change your BitWarden settings, this file would likely fill slowly and hang around on disk for a long time.

this is a pretty serious issue because users expect their master key to not be recoverable, but for a period of time (I'm going to guess days to weeks) it is.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

We are planning to change this setting's default in an upcoming release so it will be resolved going forward. That said, bitwarden syncs and overrides the user's entire encrypted vault to chrome.storage multiple times per day so I imagine that log fills up and rolls over quickly. In any case it's not there forever so I don't think we have to worry about trying to come up with a solution to cleanup those log files.

@subdavis

This comment has been minimized.

Copy link
Author

commented Jan 18, 2018

multiple times per day

For active users, maybe. But I don't think it makes a great deal of sense to store both the lock and the key in the same file on disk.

You'd be relying on users's activity to overwrite a file that contains the most sensitive piece of information the application deals with. Imagine a scenario where I install BitWarden, port over my data from LastPass, and then get sidetracked for a week. No sync, no updates, no fresh data. Then I lose my laptop on the train and there's a good chance both my encrypted vault and the key to unlock it are in one convenient location on disk.

@anortiz08

This comment has been minimized.

Copy link

commented Jan 18, 2018

@subdavis @kspearrin Should this be posted on Hacker One for tracking? https://hackerone.com/bitwarden?view_policy=true

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

It doesn't need to be since we have it here, but can be if @subdavis would like to post it there. We'll have this patched for next release.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

@subdavis

This should resolve it going forward: 4a34f9b . This will be bundled with next release which is going out rather soon.

Thanks for the additional info about chrome.storage. Never knew it behaved in such a way. Doesn't seem Google has that documented anywhere.

@kspearrin kspearrin closed this Jan 18, 2018

@subdavis

This comment has been minimized.

Copy link
Author

commented Jan 18, 2018

@kspearrin @anortiz08 That isn't a fix for the issue I've described, and changing the default won't help.

Any time a user uses the "Never" feature, a copy of key gets persisted to disk, potentially forever. This issue should be left open to document that.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

Short of removing the option to set "Never" the issue will always be present, however, the option is there by user demand so we cannot just remove it. Perhaps we should add a popup warning when setting that option so they user knows the consequences of their actions. We are tracking that in #467

@subdavis

This comment has been minimized.

Copy link
Author

commented Jan 18, 2018

Then should I add documentation to that issue?

Because the popup would have to say:

If you use this option, you won't be able to change your mind later. Short of uninstalling BitWarden, there is no programmatic way to verify that the log has rolled over and your credentials are safe again.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

Yes, you can add input regarding that in #467 .

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

Also, are you aware if other chrome.storage implementations such as Firefox or Edge have a similar log? That would determine if we would need to display the same message there or not. I can't find anything in Firefox.

@subdavis

This comment has been minimized.

Copy link
Author

commented Jan 18, 2018

I'm not. I'm planning to open an issue on Chromium to track why the hell this hasn't been documented because it's obscenely dangerous, and will attempt to do the same for Firefox, Edge, etc.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2018

I don't know why they can't provide a secure disk storage medium in the first place. Safari does (presumably linked to the keychain), which we use for the key if using the "Never" option.

@anortiz08

This comment has been minimized.

Copy link

commented Jan 18, 2018

@subdavis I understand a workaround/mitigation has been put in place but I think a formal vulnerability report should be submitted on HackerOne since this is a pretty serious vulnerability.

@FatOrangutan

This comment has been minimized.

Copy link

commented Jan 19, 2018

Hi, could somebody tell me if Bitwarden on Firefox is affected too? Thanks.

@kspearrin

This comment has been minimized.

Copy link
Collaborator

commented Jan 19, 2018

@FatOrangutan No, Firefox does not appear to be affected.

@subdavis This should be a better fix going forward 6834e26

@kriswilk

This comment has been minimized.

Copy link

commented Jan 19, 2018

I mentioned this on reddit but considering this is the official place to track the issue I will repeat it:

Issue #467 addresses the need for UI warning when selecting the "never" option...that remains on the to-do list.

But if someone DOES choose "never" and later switches to a different option, data would potentially remain cached on disk for some time.

So I think a complete fix would need code which flushes the local storage whenever the lock option is changed away from "never" to anything else. Correct?

@subdavis

This comment has been minimized.

Copy link
Author

commented Feb 8, 2018

@kriswilk You're completely correct. It's sort of an edge case, but definitely a risky one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.