6

Recently Laravel 4 was updated to address a security concern: there was a CSRF vulnerability in their code.

Here's the old code:

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

And here's their fix (note the !==):

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

I understand the difference between == and === in PHP (basically the latter is more strict because it checks type), and I understand what CSRF is and how to address it, but I don't fully understand why this specific case creates a vulnerability, or how an attacker would exploit it.

Jason
  • 63
  • 3
  • 2
    I don't know Laravel that well, but is there any chance that Input::get('_token') can return something that is not a string, like the boolean TRUE? TRUE is == to all strings, for example. – David Meister Nov 24 '14 at 13:19

2 Answers2

7

chrismsnz here...

Long story short, DarkLighting is pretty much right.

Input::get() usually reads from request parameters, but if the request is JSON then it reads from the JSON body. JSON allows you to specify the type of data so instead of a CSRF token string, I sent an int(0) which will pass that loose comparison most of the time.

The other trick is that usually you cannot send JSON requests cross-site, due to the CORS check in browsers. However, Laravel has a very poor JSON check, it basically checks to see if the string '/json' is anywhere in the content type and if its there, runs the whole request through a JSON parser and feeds it into Input.

Therefore you can exploit it using something like this (jquery example)

$.ajax("http://<laravel app>/sensitiveaction", {
    type: 'post',
    contentType: 'application/x-www-form-urlencoded; charset=UTF-8; /json',
    data: '{"sensitiveparam": "sensitive", "_token": 0}',
});
Chris S
  • 321
  • 2
  • 2
  • Thanks Chris, for the explanation. One thing that i wanna ask is this: why did int(0) match the session token? You put that as a literal zero or just an example? If that is a literal, shouldn't the server-stored session token value be different than that? – DarkLighting Feb 19 '15 at 12:19
  • Yep, this is what the Laravel patch addresses. PHP weak comparisons are pretty broken... try running this code and it should provide some insight: `` – Chris S Feb 19 '15 at 23:05
  • Ok, now i got it. According to [this link (paragraph right after the table)](http://php.net/manual/en/language.operators.comparison.php), if you compare a string and a number, the string will be converted to a number and the comparison will be numerically made. Man, this is SOOO counterintuitive... Thanks again for taking the time to enlighten us, Chris. – DarkLighting Feb 20 '15 at 13:18
5

It looks like it was a private disclosure, so it seems that we will not know this for sure unless someone takes the time to analyze the framework code (or @chrismsnz decides to explain it to us).

But from what i could tell, it seems like Session::token() returns a string and Input::get() returns a mixed object.

By the rather short explanation they gave, the researcher found a way to make the variable contain arbitrary data using JSON, but the value did not have the correct type (string). As the framework was only checking for the correct value, he was able to bypass the filter.

DarkLighting
  • 1,523
  • 11
  • 16
  • No exploit? How did it work? – rook Nov 24 '14 at 19:13
  • As i said above, it looks like the laravel framework uses private disclosures, as i was unable to find details about the flaw other the stated in their short comment on the subject. Maybe some months from now the researcher will make a public release explaining everything... – DarkLighting Nov 24 '14 at 19:32
  • Hi! I'll write this up for you below... – Chris S Feb 17 '15 at 02:42