Framework: CSRF being triggered on all routes

[mikemike]

I recently discovered that CSRF protection is triggered on all routes over POST. To me, this seems a little strange. Expected behaviour should be to only trigger on routes defined within routes.php.

I am aware that a route can be added to the $except variable within the middleware.

Because CSRF is being triggered there is quite a bit of overhead on the system versus a standard 404, so it could be a potential attack vector for a denial of service attack.

I came across this issue when an external Exchange server started bombarding our server with /autodiscover/autodiscover.xml POST requests, which flooded our New Relic error reports with false errors of CSRF token mismatch exceptions, rather than simply 404ing. More details on this exact issue can be found on Stack Overflow (http://stackoverflow.com/questions/33342784/requests-to-autodiscover-autodiscover-xml-causing-tokenmismatchexception/33366209#33366209).

Is this behaviour by design, or an unintended symptom of using middleware?

[GrahamCampbell]

This is the intended behaviour. If you don't like it, that's fine, just roll with your own csrf middleware.

[mikemike]

@GrahamCampbell, So you're saying non-existing routes throwing a token mismatch exception rather than 404 for anything other than GET requests is intended?

That seems very odd.

Surely requesting a page that doesn't exist should throw a 404, regardless of the request type?

[ghost]

Surely this could cause a ton of problems if this was exploited.

What if someone bombarded a site with POST requests, with incorrect tokens? Wouldn't this just cripple the system?

Checking if the route even exists beforehand seems a lot more logical, in my opinion.

[crynobone]

Checking if the route even exists beforehand seems a lot more logical, in my opinion.

Middleware is execute before routing is handled, unless you make it as a route middleware (which you can do so easily in Laravel).

So you're saying non-existing routes throwing a token mismatch exception rather than 404 for anything other than GET requests is intended?

You also can handle token mismatch exception from within app/Exceptions/Handler.php.

[mikemike]

@crynobone I'm not disagreeing with you, but with middleware working in the way it does, perhaps middleware isn't the best place for CSRF handling?

It doesn't make sense to call CSRF handling on every request, whether it exists or not. Handling the exceptions is not fixing the issue - the exceptions shouldn't be getting thrown in the first place.

[rkgrep]

:+1: I have to set CSRF check to route middleware in every project because of this.
Actually, the only middleware that should be handled before route resolving is maintenance mode check.

But I suppose this shouldn't be fixed in the core, but in the base laravel project.

[geoffs3310]

I agree the CSRF protection shouldn't be run on every request, as mentioned previously it makes a DDoS attack much easier. If a route doesn't exist then a 404 should be thrown, the CSRF shouldn't come into it unless its a valid route.

[mikemike]

Looks like I'm not the only one having this issue. Can this be reopened please?

[GrahamCampbell]

If you want to only validate csrf on valid routes, then don't register the middleware in the http kernel. Register it on each route you want to protect. Simple. This is NOT a "bug". This can be continued on the forums if you want to discuss solutions.

[GrahamCampbell]

Closing since a fix was merged.

[rkgrep]

Closing since a fix was merged.

10758 was reverted. Is there another fix?

[GrahamCampbell]

Hmm, ok.

[taylorotwell]

I don't really see a huge issue here. It's really a matter of preference IMO. Some people are OK with the convenience of having the CSRF as a global middleware so you never have to worry about assigning it to routes. Others may be concerned about it adding overhead or the 404 situation. It is fairly easy to move it to a route middleware in your own application if you wish.

[mikemike]

In my opinion the overhead/404 situation is very undesirable. There are lots of scenarios someone would encounter this, and the origin of the problem isn't straight forward to find. To that end I feel the default should be to have it moved to route middleware, rather than leaving devs to work out what is causing the issue.

Even a nod to this issue in the docs would be a help.

[brynj-digital]

Agree with Mike here. Any request being handled in some way other than a 404, without it being defined in the route middleware, seems very much against the grain of expected behaviour.

[ravanscafi]

Can we revive this issue?

Our team are dealing with a lot of TokenMismatchExceptions from script kiddies/bots trying to hack us, among with some real TokenMismatchExceptions and its really hard to filter out which kind is happening when we are monitoring services like New Relic.
We don't want to stop reporting this particular exception, since we often deal with cache issues that are real and demand work on our side, so this false positives bother us.

To add more context of what is happening, some bots do POSTs to URLs like azenv.php, bitrix/admin/index.php, wp-login.php, /webdav, /admin/Cms_Wysiwyg/directive/index, etc. (we can go on with this list if you want to).
Since the status is different than 404, the bots keep posting until they are banned by our firewalls (and then they change the IP and start again). This is really boring and it's not just a matter of "preference", so it would be better to have route resolution on these cases, or at least a config for it.

[taylorotwell]

Sure, open to ideas here.

[danjdewhurst]

Surely requesting a page that doesn't exist should throw a 404, regardless of the request type?

Isn't this the best default behaviour? It seems very odd for a TokenMismatchException to happen when the route doesn't even exist?