"The CSRF token is invalid." in tests after upgrading to 6.2.6 to fix CVE

[J-roen]

Symfony version(s) affected

6.2.6

Description

After installing the fix for https://symfony.com/blog/cve-2022-24895-csrf-token-fixation, tests fail with the message "The CSRF token is invalid." I have only been able to reproduce this in tests, not (yet) when running the application in the browser.

@nicolas-grekas

How to reproduce

1. Install the Symfony demo project: https://github.com/symfony/demo
2. Run ./bin/phpunit. All tests should succeed.
3. Run composer update.
4. Run ./bin/phpunit. Multiple tests should fail. Add | grep CSRF and you will find the error message.

Possible Solution

I have no clue yet. My conclusion is that $this->csrfTokenStorage->clear(); is the cause of this bug (see 5909d74#diff-0ff1412624a79146c346925f2407eb4783b144da38ddb369ca30e49d046fab70R59), but removing this is obviously not an option as it is the fix for the CVE.

Additional Context

Docker one-liner:

docker run -it --rm composer bash -c " \
  composer create-project symfony/symfony-demo my_project; \
  cd my_project/; \
  ./bin/phpunit; \
  composer update; \
  ./bin/phpunit | head -n 6; \
  ./bin/phpunit | grep CSRF \
"

[Guite]

Not sure if it is the same issue but as it sounds similar I will add my observation here:
The security fix removing CSRF tokens after a login seems to conflict with token based authentication: we've implemented an AccessTokenHandler and now $form->isValid() returns false because of a CSRF error. Possibly the same cause...

[nicolas-grekas]

@nicolasleborgne asks something similar in #49188

Am I wrong or since the security fix for CVE-2022-24894 we cannot use in combination http basic auth and csrf protection ? Tokens are cleared at each successful login and login occurred at each request so, when submitting a form, tokens do not match.

To me, all use cases that hit this were likely vulnerable to CVE-2022-24894, so we're going to have find another way to prevent CSRF, because the previous one didn't work. I'm writing "we" because I don't know if Symfony can provide something - doc or code - to cover this.

From afar, I suppose that when there is a session, the session should be used to NOT "login" at every request.

[MatTheCat]

Login from the session does not cause any issue because it is done with the ContextListener. However authenticators can ultimately cause a LoginSuccessEvent to be dispatched up to the SessionStrategyListener which will clear the CSRF token.

Symfony Demo’s tests authenticate using the HttpBasicAuthenticator on every request so when a form is submitted you’re authenticated again and the CSRF token is removed from the session.

[nicolas-grekas]

Good call: let's figure out how to fix the demo app, that will give us the food for thoughts we need here. Anyone up to give it a try?

[MatTheCat]

Using KernelBrowser::loginUser seems to do the trick 👌 but it only fixes tests…

[gnutix]

I've just upgraded symfony/security-(http|bundle) to 6.0.20 and one of our Behat test fails because of "Invalid CSRF token". I've tried doing exactly what the test is doing by clicking in the application and it works, therefore the issue is only in the tests.

To authenticate the user, we use a custom "I am logged in with email XXX" step that sets an HTTP Header, caught by a custom Authenticator.

  Scenario: ...
    Given I am logged in with email "some@email.com"
    When I go to "/some/page"
    And I fill in "some_form_field" with "some_value"
    And I press "continue_button"
    # And print last response
    Then the url should match "/some/success/page"

The "url should match" check fails because we're back on /some/page, and printing the last response shows "Invalid CSRF token" error in the HTML content.

Here's the detailed code

    /**
     * @Given I am logged in with email :identifier
     */
    public function iAmLoggedInWithEmail(string $identifier): void
    {
        $this->restContext->iAddHeaderEqualTo('Authorization', HeaderAuthenticator::AUTHORIZATION_HEADER_PREFIX . ' ' . $identifier);
    }

#[When(env: 'dev'), When(env: 'test')]
final class HeaderAuthenticator extends AbstractAuthenticator
{
    public const AUTHORIZATION_HEADER_PREFIX = 'USER';

    public function __construct(
        private UserRepository $userRepository,
    ) {
    }

    public function supports(Request $request): ?bool
    {
        $header = (string) $request->headers->get('Authorization');

        return str_starts_with($header, self::AUTHORIZATION_HEADER_PREFIX);
    }

    public function authenticate(Request $request): Passport
    {
        $headerParts = explode(' ', (string) $request->headers->get('Authorization'));
        Assert::count($headerParts, 2, 'The authorization header should have two parts: the prefix and the user identifier.');

        $passport = new SelfValidatingPassport(new UserBadge(
            userIdentifier: $headerParts[1],
            userLoader: fn (string $identifier): UserInterface => $this->userRepository->loadUserByIdentifier($identifier),
        ));

        return $passport;
    }

    public function createToken(Passport $passport, string $firewallName): TokenInterface
    {
        return new TestToken(
            interactive: false,
            user: $passport->getUser(),
            firewallName: $firewallName,
            roles: $passport->getUser()->getRoles(),
        );
    }

    public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response
    {
        throw $exception;
    }

    public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
    {
        // Let the request continue
        return null;
    }
}

[johanadivare]

I can confirm i have simmalair problem with version v5.4.19

[MatTheCat]

@nicolas-grekas would deferring the CSRF token removal on kernel.response still address the vulnerability?

[apemsel]

Confirmed on 5.4.20. When using x.509 authentication all forms stop working due to invalid CSRF token.

[apemsel]

Any advice what we should do until a proper solution is found if we have x509 and CSRF protection in production? Either pin symphony/security to a pre-patch version or disabled CSRF protection? The former might be less of a security risk?

[benr77]

Upgrading symfony/security-http (v5.4.17 => v5.4.20) gives me the exact same issue.

[MKCG]

I had the same issue, but actually it was caused by this commit symfony/http-kernel@2a91996 that resulted in the caching by the browser of some form pages containing a Set-Cookie header

[raziel057]

Same issue here following the https://symfony.com/doc/5.0/testing/http_authentication.html#using-a-faster-authentication-mechanism-only-for-tests documentation

[raziel057]

I fixed it by using the loginUser() strategy: https://symfony.com/doc/current/testing.html#logging-in-users-authentication

[MatTheCat]

Could someone confirm that #49319 fixes their issue?

[rivallinkil]

@MatTheCat #49319 fixes our issue (combination http basic auth and csrf protection) in unit test and dev env.
Thanks !

[gnutix]

@MatTheCat @rivallinkil I'd like to try your patch out in our project, but I'm unsure how to apply the patch (as it seems to be impacting multiple Symfony components/bundles).

Would you mind providing some instructions on how to upgrade my project's dependencies or apply the patch locally ?

[florimondmanca]

@gnutix I asked the same question in #49319 and I was given these resources:

You can follow https://symfonycasts.com/screencast/contributing/pr-reviewing (just skip the Creating a new Test App part as you already have an app to test on) to test a PR locally.

see https://symfony.com/doc/current/contributing/code/pull_requests.html#use-your-branch-in-an-existing-project for the way to test your change locally.

[MatTheCat]

Sooo #49194 (comment) is not a viable solution; sorry everyone..!

Maybe it is time to revive #33171 🤔

[nicolas-grekas]

Please try the fix in #49526 (it's for 5.4 but it shouldn't be that hard to adapt for 6.2)