~sircmpwn added beta 3 years ago

~ancarda 3 years ago

Alternative suggestion: implement secure cookies (i.e. set the httponly, secure, and samesite flags) and this ceases to be a problem, at-least for recent browsers.

If you want to provide protection to old browsers, you can do it without the referer header (which, afaik, can be blocked by some security/privacy browser extensions). You just hash the cookies value, and put that in a GET param. You can then statelessly check that the same cookie (sr.ht.unified-login.v1) was sent because it matches the copy given in the GET request.

In other words, to make a valid logout URL, you need the cookie - which you didn't have hence why you were trying to trick the browser into logging out. Something like this:

https://meta.sr.ht/logout?cookie=5347e64cd529802e1e3e&return_to=https://sr.ht%2F

I don't know Python, but this might be simple enough for me to get something working. I just can't find the login_user function.

~sircmpwn 3 years ago

This isn't a CSRF issue, but rather just a problem of duping someone into clicking a link like this.

~ancarda 3 years ago

Would the GET param I described above protect against that? I do believe that's a better solution than checking the referer header.

~sircmpwn 3 years ago

Well, I don't mind the idea of someone bookmarking the URL to log out without having to click our link.

~samwhited 3 years ago

It may not be a CSRF issue because you have SameSite set, but we don't have to trick users into clicking a link either. They could just visit a page with an embedded pre-load link or image, eg. visiting this issue will log you out without any user input (note that the thing that logs you out is a comment which an attacker could put in any issue):

https://todo.sr.ht/~samwhited/prefetchpoc/1

The usual way to fix this is to make the logout link a form and a POST request.

—Sam

On Tue, Sep 1, 2020, at 17:21, ~sircmpwn wrote:

This isn't a CSRF issue, but rather just a problem of duping someone into clicking a link like this.

-- View on the web: https://todo.sr.ht/~sircmpwn/sr.ht/14#event-48317

Attachments:

• signature.asc

-- Sam Whited

Florian Maury 2 years ago · edit

Oh this is very much a CSRF, even if the consequences are not dire.

Having the cookie in a GET URL is a very bad practice and I would strongly advise against it. Using a POST request would absolutely not defend against that vulnerability because those are allowed cross-origin (as long as they qualify as simple requests, according to the CORS specs).

Using cookie options (httpOnly, secure, sameSite, and renaming the session cookie to __HOST-session) would be the proper way of fixing this. Another, less standard, way is indeed to check either the Referer: or the Origin: headers. This would work, but I would advise to stick to using cookie options, which would protect to whole site, and not just one particular function.