slimphp / Slim-Csrf

Slim Framework CSRF protection middleware
MIT License
336 stars 58 forks source link

CSRF always fails in 1.3.0 #166

Closed mijorus closed 11 months ago

mijorus commented 1 year ago

https://github.com/slimphp/Slim-Csrf/blob/ebaaf295fd6d7224078d8ae3bba45329b31798c7/src/Guard.php#L240

I'm not really that skilld with Xor operators, but the middleware used to work perfectly before

PHP 8.1.4

mijorus commented 1 year ago

specifically, unmaskToken does not return a string but a byte array

akrabat commented 1 year ago

Can you provide an example of it failing please?

I've tested using https://github.com/akrabat/slim4-csrf-example in PHP 8.1 and it works for me.

Regarding Xor, both $key ^ $token & $key ^ $decodedMaskedToken result in a string as both variables are strings. You can see this here: https://3v4l.org/ETqhk.

mijorus commented 1 year ago

can't figure this out

There is probably something wrong somewhere. Now it seems that it is ignoring the $persistentTokenMode and is generating a new token on every request.

This is how I am instantiating the Guard


// Register CSRF Middleware On Container
$responseFactory = $app->getResponseFactory();
$container->set('csrf', function () use ($responseFactory) {
    $storage = null;
    return new Guard($responseFactory, 'csrf', $storage, null, 200, 16, true);
});
akrabat commented 1 year ago

Strange.

When I change my DIC definition to Guard::class => fn () => new Guard(AppFactory::determineResponseFactory(),'csrf', persistentTokenMode: true),, I can then re-submit the posted form many times, showing that it is persisting correctly.

Do you have the PHP ini setting session.cookie_secure set to on and then accessing via http:// ?

mijorus commented 1 year ago

ok, I've done my tests.

The issue is definitely here

hash_equals($token, $this->unmaskToken($value))

the result of this function is always FALSE. I can confirm that the $_SESSION is being populated as expected, and that the function is checking the right key-pair.

For example this check:

name: csrf63739e31693b5
val: 6a8a38c367810c5cda21459046511d00

returns False.

It is generating a new token because the check fails, so there is nothing of unexpected there.