Stop Messing up CSRF Protection

Note: This article doesn’t contain anything new or ground breaking. This is the stuff that you should know already because it’s been written about before by people smarter than me. Tragically, despite lots of great material on subject people keep messing it up.

So, cross site request forgeries are a pretty common topic these days; they’re in almost every security talk, book, site etc. This is okay; they’re important (but I wish people would concentrate on security as a whole rather than just worrying about problems with nifty acronyms). Most of the sites, and all of the books I’ve read demonstrate things correctly, but when it comes to actual implementation, time and time again, I see code that’s just wrong. This CSRF Demonstration page will be used throughout this post.

In order to effectively use the common transparent defense against CSRF attacks you need to generate an unpredictable token, and confirm its presence in both the session and form submission upon receipt. Only two essentials there, but I’ve seen lots of live code that fails on at least one of the two.

Comparison

So, the point of CSRF is to confirm that the CSRF token in the session is equal to the one received with the form post. This is critical and easy, though people seem to manage to screw it up. They write code that looks remarkably like this:

if ($_POST['csrf'] == $_SESSION['csrf'])

Do you see the fatal flaw? If both the session and post variable are empty, they’re also equal. So if you’re attacking the form you simply omit the post variable. I demonstrate this attack on my negligence csrf attack page. Not only do you need to ensure they’re equal, you also need to ensure that they’re both set and non-empty.

Note I’ve left the warnings in on purpose. I could have turned off display errors, or suppressed them, but I really wanted to show what was happening.

Unpredictable Token.

This part should also be easy; generate a token that the attack can’t guess. I haven’t actually said random here, though random is good. The important part is that the attacker can’t guess it. I have a piece of information for you that shouldn’t be news, but it might be. Time isn’t random. Time increases by one, once a second, every second. MD5 also isn’t random. Thanks to the snowball effect the hash of very similar values are actually very different, but they are in no way random. Now combining these two non-random values also happens to give you a non-random result. Yet, time and time again I see code exactly like this:

$csrfToken = md5(time());

This is idiotic. Throwing a hash function at the problem doesn’t solve anything. It’s obfuscation, and weak obfuscation at that.

Now I’ve mentioned this at talks before, and the audience has politely nodded, then privately told me that it doesn’t actually matter. It does, so take a look at the md5 time attack page.

Now adding a little bit of salt also doesn’t solve the problem, it’s just more obfuscation.

If you’re interested you can also see the source code for my weak csrf page.


Stefan Esser’s talk at ZendCon Lesser Known Security Problems in PHP discusses a few other issues related to sessions you may want to take a look at.

So please go home and fix your CSRF pages.

(s/messing/&$#@ing)