search

SWI-Prolog / packages-http

The SWI-Prolog HTTP server and client libraries
23 stars 23 forks source link

Add samesite attribute to session cookies #126

Closed Anniepoo closed 5 years ago

Anniepoo commented 5 years ago

This PR closes a vulnerability in sessions whereby a third site can forge authorization to REST services on a SWI-Prolog web framework server, via the Cross Site Resource Forgery(CSRF) exploit.

Most modern browsers support the SameSite attribute for Set-Cookie. Suppose a user browses good.org (a prolog site) and obtains a session cookie by logging in. The user now visits evil.com. evil.com may now make REST requests of good.swi-prolog.org via normal AJAX calls. The session cookie will be provided. If the SameSite attribute is set to lax or strict, the browser will refuse the request. Setting to lax allows cross site requests in certain circumstances (eg links) that are usually benign. Setting to strict prevents, for example, making a link to a protected swish page work even if I'm currently logged into swish. So I've made 'lax' the default. The current only choice (no attribute) is wrong in almost all cases. This PR should have minimal impact on existing users, and closes a serious security vulnerability.

JanWielemaker commented 5 years ago

Very useful. One little question though: are there cases were we want the old behavior? Or, should there be a setting value, e.g. false that causes the attribute not to be there or is there simply no way why you would not want that? Probably that is the same as asking why the default is not lax in browsers?

Anniepoo commented 5 years ago

I waffled on whether to add a 'none' option, and decided not to. In retrospect, I'm not sure why - I think I was afraid of adding a seldom/never used option. I don't like having code paths around that aren't going to be used. But ok, we're core code - I added 'none' as an option

JanWielemaker commented 5 years ago

Thanks. Merged after squashing the two commits, some layout and the commit message. Good catch!