Sessions not being destroyed when a user deletes their own account
We received a report from HackerOne of a CSRF vulnerability. It turned out not to be exploitable but it did expose some bad behavior by GitLab relating to creation and deletion of accounts.
Here was the vulnerability:
If a user deletes their existing account and immediately creates a new account (regardless of whether email confirmation is enabled), the csrf_token
from the first account remains valid for the new account. This could theoretically allow an attacker to record their own csrf_token
and then leave a browser session open for an unsuspecting victim who registers an account. The attacker would then have a valid csrf_token
to use to hijack the victim's account.
This relies on a lot of very unlikely circumstances involving sharing a browser session between attacker and victim. This does not work if the victim logs into an existing account, as Gitlab is configured to require Warden to create a new csrf_token
upon authentication. This is designed to protect against CSRF token fixation attacks. The problem is that GitLab doesn't force users to login when verifying a new account, even with email confirmation enabled: https://gitlab.com/gitlab-org/gitlab-ce/issues/24411
There are two related issues here:
- GitLab doesn't destroy a user's session when they delete their account. They are de-authenticated but their session ID and
csrf_token
remain valid. - GitLab doesn't force users to authenticate using their password before using the application with newly created accounts. Therefore the
after_authentication
code from Warden is never executed to reset theircsrf_token
.
The first is an easy fix, just call session.destroy
from inside the registrations controller's destroy
method and a new session ID and csrf_token
will be created when they get redirected to the sign_in page. I've verified that this code is not called when an admin deletes a user account. This doesn't solve the overall problem because the attacker can still steal the new csrf_token
, but it is probably good practice anyways.
The second is probably best fixed by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7472 (requires users to login to new accounts using a password) but could also be fixed by recreating the session in the create method of the registrations controller. It would also need to be done inside the email confirmation method for sites requiring email confirmation. I still say forcing a user to login when clicking a confirmation link is the best route to take.