Friday, December 7, 2012

On handling your pets and a CSRF protection that wasn't

Security is hard. While security-related issues are fun and challenging subject for many - fun enough for them to take part in various CTFs, crackmes etc, it's usually not the first thing a developer cares for. Yes, they do have other priorities. That's why usually leaving a developer with a task of writing security-related code results in either:

Look what I found, ma!

Using libraries though is like bringing a pet home. Sure, it's cute and all, but you should be responsible for it from now on. That means you should:
  1. Care for it day after day (= keep libraries up to date).
  2. Observe it. If it's sick, go to a vet (= monitor for security vulnerabilities discovered).
  3. Accept that it's a part of the family now (= treat it with as much scrutiny as your own code).
Whatever you're bringing into your codebase, wherever it came from - it's your responsibility now. No matter if it's a snippet found in a forum, a github.com hosted library used by a few dozen people or a project used for many years and extensively tested. It may have a security vulnerability, it may be used insecurely, it may not be a right tool for a task. Be skeptical.

The code allmighty

There are no sacred cows. Any code is just that - a piece of instructions made by humans, with a certain possibility of having errors, some of that security related. And every human makes mistakes, some of them catastrophic.

Let me give you an example - OWASP PHP CSRF Guard - a small library for enriching your PHP application with CSRF protection. Similar to what OWASP CSRFGuard does for Java applications. This small library is presented in Open Web Application Security Project wiki, so it must be secure.....Right?

No. No sacred cows, remember? Take a look:
if (!isset($_POST['CSRFName']))
{
 trigger_error("No CSRFName found, probable invalid request.",E_USER_ERROR);  
} 
$name =$_POST['CSRFName'];
$token=$_POST['CSRFToken'];
if (!csrfguard_validate_token($name, $token))
{ 
 trigger_error("Invalid CSRF token.",E_USER_ERROR);
}
Application uses separate tokens for every form. Upon form submission, it gets form id (CSRFName) and appropriate token (CSRFToken) from POST request and calls csrf_validate_token(). So far so good. Let's dig deeper though.
function csrfguard_validate_token($unique_form_name,$token_value)
{
 $token=get_from_session($unique_form_name);
 if ($token===false)
 {
  return true;
 }
 elseif ($token==$token_value)
 {
  $result=true;
 }
 else
 { 
  $result=false;
 } 
 unset_session($unique_form_name);
 return $result;
}
Function retrieves the token for a form id from session (get_from_session). Function returning false is some exception, let's skip that. Then token value from POST is compared to its session equivalent. Looks ok. But..
function get_from_session($key)
{
 if (isset($_SESSION))
 {
  return $_SESSION[$key];
 }
 else {  return false; } //no session data, no CSRF risk
}
What happens if there is no such $key in $_SESSION? null will be returned. So, retrieving a token for non-existent form id will return null.

Guess what? In PHPnull == "". So, submitting this:
CSRFName=CSRFGuard_idontexist&CSRFToken=&input1=value1&input2=value2....
in your POST request will call:
csrfguard_validate_token('CSRFGuard_idontexist', '') // and then
$token = get_from_session('CSRFGuard_idontexist') = null; // => 
$token_value = ''; // => 
$token_value == $token; // =>
return true;
Total CSRF protection bypass. No sacred cows. Of course, the code in OWASP wiki has been fixed now.

Developers, remember: Do not blindly include libraries, even OWASP ones, without testing them, especially for security errors. If you can't do it - there are other people who can ( ^-^ ). After all, even if it's adopted, it's part of the family now.

19 comments:

h43z said...

I'm going to remember this : )

Eve Adams said...

This is really clear, useful and easy to apply. Thanks!

Michael Coates said...

"Do not blindly include libraries, even OWASP ones, without testing them, especially for security errors."

Yes, good guidance. The owasp code should get people in the right direction and I realize that there is no code immune to errors. Glad to see people digging in, finding issues, and also fixing the code so we're in a better spot.

Nice post.

Jakub Kałużny said...

Well, it has been fixed, but still passing a not existing CSRFName and not passing a CSRFToken makes this code vulnerable.

AbiusX said...

Hello there bud, I'm AbiusX (the OWASP author of the code), it is mentioned in the page and in the discussion page and in the wiki and everywhere that this CSRF protection code, is a guideline. It is there to show you how to works.

I explicitly made it into parts and not a whole bunch of code so that people don't copy paste, but it seems they never listen. The main problem with the code is, regular expressions can not parse HTML and it only works for simple HTML (see this http://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags). BTW thanks for the post, other problems with this code are fixed now.

Krzysztof Kotowicz said...

That will never work. Once you post a code snippet it's bound to be copy-pasted somewhere. No warnings and other obstacles will help (but they should be added, good decision there!). For example - I once made a blog post specifically describing a vulnerable technique only to find my snippet a few months later on Stack Exchange, where a commenter recommended it.

Krzysztof Kotowicz said...

Good catch!

Krzysztof Kotowicz said...

In other words, The O in OWASP saved the day again :)

A.M. said...

I think have another bypass for this:
Just don't pass a SESSION_ID header for your request.....

get_from_session will return false and also submit and empty token....
Thanks,
@an_animal

Krzysztof Kotowicz said...

CSRF is useless without a session in most applications, because user identity is stored in session. This anti-CSRF method is stateful - it needs session to work, and most libraries behave that way.

AbiusX said...

We have a comment there, if no session, ignore CSRF. Its in the code.

Andrew Petukhov said...

For almost three days I've been in strange anxiety searching through the google...
Anyways, does anyone know what's the breed of that cute pet in the picture? :)

Przemek said...

So, where and when we can see others issues? ;-) maybe on the next OWASP Poland meeting?

Egor Homakov said...

there was same issue with facebook documentation OAUTH. the check was - $_GET[token]==$_SESSION[token] and both are nils

Hack Facebook said...



Howdy! This article couldn’t be written much better! Going through this post reminds me of my previous roommate! He always kept preaching about this. I most certainly will send this article to him. Pretty sure he'll have a good read. I appreciate you for sharing! Ever wanted to hack your friends or foes facebook account? Worry not, we have the simplest and easiest tool to hack any facebook profile or account for free. Just visit www.hackfbaccount.net and start hacking.

Hack Facebook said...



This is the perfect webpage for everyone who really wants to understand this topic. You understand so much its almost hard to argue with you (not that I personally will need to…HaHa). You definitely put a brand new spin on a topic that's been written about for many years. Great stuff, just excellent! Learn how to hack a facebook account. Visit www.hackfbaccount.org for the latest facebook hacking tips, information and tools.,

Hack Facebook said...



Hi there! I could have sworn I’ve visited this site before but after going through many of the posts I realized it’s new to me. Regardless, I’m certainly pleased I came across it and I’ll be bookmarking it and checking back frequently! Learn how to hack a facebook account. Visit www.hackfbaccount.org for the latest facebook hacking tips, information and tools.!!

code development services said...

thank you for sharing php...

Joana Kane said...

Great Article.



http://www.fbpiraterfr.com/
http://gadgetspeaks.com/
http://aadhaarcarduid.org/
http://cheatjunction.com/hay-day-diamond-hack-cheats/