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:

• total fail,
• major fail,
• minor fail, 
• or using well-respected libraries for the task.

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 PHP, null == "". 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.