clients icon indicating copy to clipboard operation
clients copied to clipboard

[SECURITY] Browser extension sends access_token within GET request

Open rwjack opened this issue 2 years ago • 4 comments

Steps To Reproduce

  1. Enable websocket notifications on my server

Expected Result

  1. No user information disclosure on proxies that route traffic to the bitwarden server instance

Actual Result

  1. GET request contains user's access token along with encoded/encrypted data

Screenshots or Videos

No response

Additional Context

It would be better if the extension would send POST requests to the server, since the entire GET request with a user's access_token is stored in the proxy access logs.

Operating System

Linux

Operating System Version

No response

Web Browser

Firefox

Browser Version

No response

Build Version

2022.12.0

Issue Tracking Info

  • [X] I understand that work is tracked outside of Github. A PR will be linked to this issue should one be opened to address it, but Bitwarden doesn't use fields like "assigned", "milestone", or "project" to track progress.

rwjack avatar Dec 21 '22 12:12 rwjack

It would be better if the extension would send POST requests to the server,

IMO tokens should be sent as HEADER, but maybe that's just a personal preference. However, I believe I read many moons ago that authentication tokens are supposed to be sent via Header.

tessus avatar Dec 22 '22 20:12 tessus

@tessus Yes, that seems like a better solution. Anything would actually be better than a GET request.

rwjack avatar Dec 23 '22 07:12 rwjack

Just found this issue cause I was looking for something else.

Just wanted to let you know that sending the access token via query parameter is actually the way to go for websockets. That's a browser limitation. There is no way to specify a Header or a POST request when estabilishing a websocket connection via browser.

See also this documentation for SignalR by microsoft:

https://learn.microsoft.com/en-us/aspnet/core/signalr/authn-and-authz?view=aspnetcore-7.0

"The query string is used on browsers when connecting with WebSockets and Server-Sent Events due to browser API limitations. When using HTTPS, query string values are secured by the TLS connection. However, many servers log query string values. For more information, see Security considerations in ASP.NET Core SignalR. SignalR uses headers to transmit tokens in environments which support them (such as the .NET and Java clients)."

Also some more options (e.g. cookie maybe or sending authentication via the first message) are explained here: https://websockets.readthedocs.io/en/stable/topics/authentication.html

WolfspiritM avatar Dec 24 '22 00:12 WolfspiritM

Well how about that, you learn something new every day.

The next possible solution I'm thinking of would be to sed the offending lines before rotating the log, but that's 24 hours too long for a compromising query to be sitting on disk.

I haven't really investigated much, but I'm guessing this access token can easily be reused if the proxy server is compromised.

Also, classic Microsoft solution: "Have issues? Just disable logging", yeah thanks MS.

Since I am using Traefik as a proxy, this sounds like a potential missing feature, not to log regex matching logs. I'll link the issue here when I create it, for future reference.

Other than that, not sure if there's anything else that can be done about this, so I'll keep the issue for some time, to stimulate further discussion.

rwjack avatar Dec 24 '22 03:12 rwjack

@rwjack As we use GitHub issues as a place to track bugs and other development related issues and there is no action for the engineering team, I am going to close this now.

For further discussion around this topic, please feel free to open a thread on the Bitwarden Community Forums. We have a lot of very engaged community members there, that might have a similar scenario or thoughts on this.

Kind regards, Daniel

djsmith85 avatar Jan 09 '23 21:01 djsmith85