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
- GitLab doesn't force users to authenticate using their password before using the application with newly created accounts. Therefore the
after_authenticationcode from Warden is never executed to reset their
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 !7472 (merged) (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.