PHP CSRF Token code. Sufficient security?

Having recently protected my site the best I can against XSS I am now in the process of protecting against CSRF, having watched and read some articles on the matter I have created the below code.

I would like to know whether my implementation is correct in using a string in the database to help security. Is the anything I should be doing differently?? Should I be checking the database on both sides???

Code:

if(!isset($_SESSION['register_token'])){

  $keytype = 'register';

  $getregisterkey = mysql_query("SELECT key FROM tokenkeys WHERE type='".$keytype."' ") or die(mysql_error()); 
  while ($row = mysql_fetch_array($getregisterkey))
  {
$registerkey = mysql_real_escape_string($row['key']);
  }; 


  $_SESSION['register_token']=sha1(uniqid(rand(), TRUE).$registerkey);
  $_SESSION['register_token_time']=time();
}

<input type="hidden" name="token" value="<?php echo $_SESSION['register_token'];?>" />



if($_POST['register_token']==$_SESSION['register_token']){
  $register_token_age=time()-$_SESSION['register_token_time'];
  if($register_token_age=>300){

    //process form
  } else{
    //valid token but expired
  }
} else{
  die('Access Forbidden')

}

Solution

XSRF tokens are only as safe as the channel over which the page is sent. Use https and only https on this form and only submit it to an https endpoint. Otherwise a MITM can get your XSRF token from the form as it is served, or as it is submitted.

while ($row = mysql_fetch_array($getregisterkey))
{
    $registerkey = mysql_real_escape_string($row['key']);
};

will never execute when there are zero rows which case you don't get much entropy from $getregisterkey when you later do

$_SESSION['register_token']=sha1(uniqid(rand(), TRUE).$registerkey);

so I would make sure that your implementation fails-fast if there are zero rows returned. Maybe change to if ($row = mysql_fetch_array(...)) { ... } else { /* abort */ } since you get no benefit from extra rows.

The rand() needs to be either truly random or a cryptographically strong PRNG.

I am not familiar with PHP's standard libraries but [wikipedia] suggests rand() is not cryptographically strong. wikipedia says

There are proposals for adding strong random number generation to PHP.

Strong cryptography in PHP suggests using openssl_random_pseudo_bytes()

Don’t use rand() or mt_rand()

To generate a cryptographically strong random number in PHP you have to use the function openssl_random_pseudo_bytes() of the OpenSSL library.

If you use weak randomness then an attacker can observe the numbers you generate (by requesting multiple version of the form and parsing out the hidden input) and use that to figure out what the next numbers might be and forge CSRF tokens.

If an attacker modify the 'register_token_time' session property then they can avoid your XSRF checks.

For example, if you have a page that does

$_SESSION[$_POST['x']] = $_POST['y'];

then an attacker can POST

x=register_token&y=pwnd

to replace the register_token stored in the session and then send a post with

token=pwnd

and bypass your XSRF protection.