Skip to content
Permalink
Browse files
Check type of token as well.
  • Loading branch information
taylorotwell committed Nov 9, 2014
1 parent 5eb4d0a commit ba0cf2a1c9280e99d39aad5d4d686d554941eea1
Showing with 1 addition and 1 deletion.
  1. +1 −1 app/filters.php
@@ -83,7 +83,7 @@

Route::filter('csrf', function()
{
if (Session::token() != Input::get('_token'))
if (Session::token() !== Input::get('_token'))
{
throw new Illuminate\Session\TokenMismatchException;
}

7 comments on commit ba0cf2a

@lasselehtinen

This comment has been minimized.

Copy link

@lasselehtinen lasselehtinen replied Nov 10, 2014

Isn't this still theoratically vulnerable to timing attacks since !== and strcmp are not executed in constant time? Only things I know to fix this, PHP 5.6 would have hash_equals and Symfony has this:

https://github.com/symfony/symfony/blob/v2.5.6/src/Symfony/Component/Security/Core/Util/StringUtils.php#L28-L65

@ircmaxell

This comment has been minimized.

Copy link

@ircmaxell ircmaxell replied Nov 10, 2014

Yes, this is still vulnerable to timing attacks identifying the csrf token.

Suggest you use a timing safe comparison function.

@barryvdh

This comment has been minimized.

Copy link
Contributor

@barryvdh barryvdh replied Nov 11, 2014

@ircmaxell so do you recommend using the Symfony version (which is always available to Laravel) or do you have a alternative / forward compatibility library/ plans to add that to password_compat? ;)

@barryvdh

This comment has been minimized.

Copy link
Contributor

@barryvdh barryvdh replied Nov 11, 2014

I've created 2 PR's (for L4 and L5), would that suffice?
Laravel 4: #3126
Laravel 5: laravel/framework#6385

@lasselehtinen

This comment has been minimized.

Copy link

@lasselehtinen lasselehtinen replied Nov 11, 2014

You ended up using the Symfony component, makes sense. But looks good to me, thanks!

@erguncaner

This comment has been minimized.

Copy link

@erguncaner erguncaner replied Nov 11, 2014

@barryvdh

This comment has been minimized.

Copy link
Contributor

@barryvdh barryvdh replied Nov 11, 2014

Please sign in to comment.