CSRF

[Destramic]

hey guys,

 

i was introuduced the the world of csrf a little while ago by a member of PHP Freaks, beofore hand i had'nt a clue...so i decided to read a little more into and created a class to deal with generating tokens and ensuring the site is free from CSRF.

 

now my understanding is that a CSRF can be made from clicking on sponsers, images and basically anything that can cause a request to another site/domain.

 

now with the script allows the user to have multipule tokens and a new token is generated everytime when filling a form or whatever, allowing user to have more than one tab open.  I'm just a little concerned that a CSRF attack can still be made this way as a new token is made on each form page.

 

when creating a form i do this:

<input name="csrf_token" type="hidden" value="12345" />

then on post im able to do something like this:

$token = $csrf->get_token(); // token for input

if ($csrf->is_safe($post->csrf_token) && form->is_valid())
{
	echo "safe"
}
else
{
	echo "unsafe";
}

here is my class

<?php

namespace Security;

use Session\Session as Session;
use Security\SSL;

class CSRF
{
	protected $_expiration = "3600";
	
	public function get_token($expiration = null)
	{
		$ssl     = new SSL;
		$token   = $ssl->random_string(20);
		$session = new Session;
		$session->start();
		
		if ($expiration === null)
		{
			$expiration = $this->_expiration; 
		}
		else if (!is_numeric($expiration))
		{
			// error	
		}
		
		if (!$session->offset_exists('csrf_token'))
		{
			$session->csrf_token = array();
		}
		
		$expiration = time() + $expiration;
		
		$session->append('csrf_token', array('token'      => $token,
				                             'expiration' => $expiration
		));
		
		return $csrf_token;
	}
	
	protected function token_exists($token) 
	{
		$session    = new Session;
		$session->start();
		$csrf_token = $session->csrf_token;
		$result     = false;
		
		foreach ($csrf_token as $key => $array)
		{
			if (time() > $array['expiration'])
			{
				$session->offset_unset('csrf_token', $key);
			}
			else if ($array['expiration'] > time()&&
			         $array['token'] === $token)
			{
				$session->offset_unset('csrf_token', $key);
				$result = true;
			}
		}
	
		return $result;
	}
	
	public function is_safe($token)
	{
		if ($this->token_exists($token))
		{
			return true;
		}
		
		return false;
	}
}

any advise would be greatful, thank you

Edited January 11, 2015 by Destramic

[Jacques1]

If the random numbers are sufficiently strong, I don't see any major security problems.

 

However, maintaining a long list of all current tokens and iterating through it on every single request isn't really a good approach. You should generate one token in the log-in procedure and then simply use this token for every form.

 

Generating a new token for every request is acceptable and secure, but it has no advantage over a single token and is unnecessarily complex. Should an attacker manage to get one token (through cross-site scripting or network sniffing), then it's game over either way. Invalidating the token on first use won't help. So you might as well use a single token for the entire session.

 

The expiration is also problematic. Let's say I write a long text, maybe leave the PC for a while and then try to submit the text, but the token has already expired. Will I lose the entire text? In your model, it's an “illegal” request. But of course there's no attack whatsoever, I just needed more time than you expected. You wouldn't have this problem if the token was valid for the lifetime of the session.

[Destramic]

Your right...I've just made something so simple complex...that's what you get for looking a other people's ideas and thinking it's the best way. I'm gonna remove most of the crap and just have it as you say...one token for a whole session...thanks again for your help