Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSRF regenerate being called when csrf is disabled #16665

Open
steve-obrien opened this issue Aug 24, 2018 · 4 comments
Open

CSRF regenerate being called when csrf is disabled #16665

steve-obrien opened this issue Aug 24, 2018 · 4 comments
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug

Comments

@steve-obrien
Copy link

What steps will reproduce the problem?

  1. Create a form with CSRF enabled.
  2. Before the form is submitted create a POST/GET javascript call to a controller that implements an auth method that invokes $user->loginByAccessToken() (this controller also intentionally sets enableCsrfValidation = false and disables the session)
  3. Submit the form

What is the expected result?

The form should successfully submit and CSRF validation should pass

What do you get instead?

CSRF validation fails

Additional info

Q A
Yii version 2.0.14.1+
PHP version 7.1.1 (NA)
OSX High Sierra (NA)

The problem I think is caused by line 261 of \yii\web\User that calls$this->regenerateCsrfToken();
The javascript API call invokes $user->loginByAccessToken which triggers the regeneration of the CSRF token and invalidates the one on the form that has yet to be submitted.

I believe the solution would be to add a conditional statement to check if csrf validation is enabled for the current action.

if (Yii::$app->getRequest()->enableCsrfValidation)
    $this->regenerateCsrfToken();

or

if (Yii::$app->controller->enableCsrfValidation)
    $this->regenerateCsrfToken();
@samdark samdark added type:bug Bug status:ready for adoption Feel free to implement this issue. labels Aug 25, 2018
@koteq
Copy link
Contributor

koteq commented Sep 4, 2018

Regeneration of CSRF token was introduced by #15496. Maybe it's a wise idea to let the developers to opt out from the enforced token regeneration with some kind of a switch. Should it be a public var on yii\web\User class?

Also I have to say, that I don't see any benefits of having this kind of enforced token renewal.

@samdark
Copy link
Member

samdark commented Sep 6, 2018

If fixes security issue that was confirmed.

@SamMousa
Copy link
Contributor

SamMousa commented Sep 7, 2018

Note that while unrelated, my proposal in #16214 would also fix (the symptoms of) this issue.

@koteq
Copy link
Contributor

koteq commented Sep 7, 2018

If fixes security issue that was confirmed.

Found it. CVE-2018-6009: In Yii Framework 2.x before 2.0.14, the switchIdentity function in web/User.php did not regenerate the CSRF token upon a change of identity.

Also there is a soft-deleted issue #15147.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug
Projects
None yet
Development

No branches or pull requests

4 participants