finos / symphony-bdk-java

The Symphony BDK (Bot Developer Kit) for Java helps you to create production-grade Chat Bots and Extension Applications on top of the Symphony REST APIs.
https://symphony-bdk-java.finos.org
Apache License 2.0
23 stars 69 forks source link

Cookie set without SameSite=Strict #483

Closed andysymphony closed 3 years ago

andysymphony commented 3 years ago

Bug Report

The request to /bdk/v1/app/auth appears to be vulnerable to cross-site request forgery (CSRF) attacks against authenticated users.

The request can be issued cross-domain. The BDK relies solely on HTTP cookies to identify the user that issued the request. The request performs some privileged action within the application, which returns a token. The attacker can determine all the parameters required to construct a request that performs the action. If the request contains any values that the attacker cannot determine or predict, then it is not vulnerable.

Steps to Reproduce:

  1. .Send an HTTP request to /bdk/v1/app/auth by modifying the Referer (see below) image
  2. The request passes and the token is obtained: image

Expected Result:

Should return a 401 (Not authenticated), because the Cookie should not be sent (auth cookie not sent = not authenticated)

Actual Result:

The request passes through, because of the Cookie which is sent, due to the fact that it does not contain the Strict value for Samesite attribute (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#strict).

Environment:

None

Additional Context:

None

Recommendation:

Set the Strict value to the Samesite attribute. This will prevent sending the cookie when the request is done from another domain.

symphony-youri commented 3 years ago

/bdk/v1/app/auth does not use the Cookie value sent Anybody can call this endpoint (the BDK then calls https://developers.symphony.com/restapi/reference#application-rsa-authentication from the POD which is open to anyone if I'm not wrong) If I understand CoT correctly the returned token does not give access to anything but rather let's you continue the CoT for each party to validate that it talks to the right person

About cookie we had this issue that lead to opening cookies: https://perzoinc.atlassian.net/browse/APP-3271 Looking at it again I wonder if this is something that should only be done locally when testing with a backend/frontend running on different domains Usually I would expect the extension app and its backend to be on the same domain.

gisovanloon commented 3 years ago

Indeed, the question is whether it is a valid use case to have extension app and backend running on different domains (which in my view it is).

As I understand it, to ensure the end points are not opened up to the world, the cors settings can be used. In that way it can be restricted to a single domain. By setting SameSite to Strict this becomes redundant. To tighten security you can set SameSite to Strict in case the bdk-app.cors.allowed-origins setting is not an array of specific domains.

andysymphony commented 3 years ago

The problem is that if the extension app has a misconfigured cors, the CSRF vulnerability is very serious. That is also the reason why Google decided to default behaviour to SameSite=Strict starting with February 2020

gisovanloon commented 3 years ago

Exactly - the vulnerability occurs when the developer has misconfigured the CORS settings. I noticed that in the example there was no Access-Control-Allow-Origin header, nor any other access control headers.

In the documentation of the BDK the cors set up is explained, giving the impression that having different domains for the back end and the extension app is not unusual. A developer might find it then confusing that even if they have set it up correctly, that this cookie will still not be set. To protect against an incorrect cors configuration, should there not rather be a check to ensure the allowed-origins is set (and not set to "*"), and if not then revert to Strict?

Incidentally, as I understand it Chrome's default behaviour is SameSite=Lax (see https://www.chromestatus.com/feature/5088147346030592 )