I'm also using bitwarden, but palent seemed skeptical about it in the comments. Here's a copy of the comment:
Reply from Wladimir Palant:
Unfortunately, I didn’t make notes last time I looked into this – the issues simply weren’t serious enough for reporting. And I only looked at a small portion of the codebase, so when I look at it now it will probably be some different code paths. So the getDomain() function I see under https://github.com/bitwarden/jslib/blob/dd46d5ecdd51f91dace5... is indeed using URL objects. It also knows that tld.js won’t handle IP addresses correctly, but it will only consider IPv4 addresses in dotted decimal notation and not IPv4 addresses in other notations or IPv6 addresses. All of that appears to be a minor risk but not an actual issue – assuming that URLs are already normalized when they get here (ok, let’s ignore the code prefixing URLs with http:// here).
The code at the bottom of this function is quite problematic however. Rather than ignoring non-HTTP URLs, this function will pass them to tld.js. But tld.js isn’t aware that non-HTTP URLs can have different semantics, so it will happily return “example.com” when it is fed something like “data://example.com,asdf/”. Oops, I think that one might even be exploitable…
I think I’m going to stop here. This needs a structured effort, not spending ten minutes every now and then. As I said, the codebase isn’t bad. But there are obvious issues that shouldn’t have been there. As always, spotting the issues is the easy part – proving that they are exploitable is far harder. I’m not going to spend time on that right now, so let’s just file these under “minor quality issues” rather than “security problems.”
I’ve also seen it recommended by Troy Hunt (haveibeenpwned creator): https://www.troyhunt.com/password-managers-dont-have-to-be-p...
(I’m using bitwarden myself, couldn’t justify the subscription cost for my usage)