PHP security: 'Nonce' or 'unique form key' problem...

Question

Hi, I use this class (taken from a blog tutorial) to generate unique keys to validate a form:

class formKey {
    //Here we store the generated form key
    private $formKey;

    //Here we store the old form key
    private $old_formKey;

    //The constructor stores the form key (if one excists) in our class variable
    function __construct() {
        //We need the previous key so we store it
        if(isset($_SESSION['form_key'])) {
            $this->old_formKey = $_SESSION['form_key'];
        }
    }

    //Function to generate the form key
    private function generateKey() {
        //Get the IP-address of the user
        $ip = $_SERVER['REMOTE_ADDR'];

        //We use mt_rand() instead of rand() because it is better for generating random numbers.
        //We use 'true' to get a longer string.
        $uniqid = uniqid(mt_rand(), true);

        //Return the hash
        return md5($ip . $uniqid);
    }

    //Function to output the form key
    public function outputKey() {
        //Generate the key and store it inside the class
        $this->formKey = $this->generateKey();
        //Store the form key in the session
        $_SESSION['form_key'] = $this->formKey;

        //Output the form key
        // echo "<input type='hidden' name='form_key' id='form_key' value='".$this->formKey."' />";
        return $this->formKey;
    }

    //Function that validated the form key POST data
    public function validate() {
        //We use the old formKey and not the new generated version
        if($_POST['form_key'] == $this->old_formKey) {
            //The key is valid, return true.
            return true;
        }
        else {
            //The key is invalid, return false.
            return false;
        }
    }
}

Everything in my website goes trough index.php first, so I put this in index.php: $formKey = new formKey();

Then, in every form I put this: <?php $formKey->outputKey(); ?>

That generates this: <input type="hidden" name="form_key" id="form_key" value="7bd8496ea1518e1850c24cf2de8ded23" />

Then I can simply check for if(!isset($_POST['form_key']) || !$formKey->validate())

I have two problems. First: I cant use more than one form per page becouse only the last key generated will validate.

Second: Because everything goes trough index.php first, if I use ajax to validate the form, the first time will validate but the second time not, because index.php generates a new key but the pages containing the form does't refresh so the form key is not updated..

I have tried several things but I cant get it to work.. Maybe YOU can update/modify the code/class to get it to work?? Thanks!!!

Answer 1

Not tested, but it should work.

class formKey {
    //Here we store the generated form key
    private $formKey;

    //Here we store the old form key
    private $old_formKey;

    //The constructor stores the form key (if one excists) in our class variable
    function __construct() {
        //We need the previous key so we store it
        if(isset($_SESSION['form_key'])) {
            $this->old_formKey = $_SESSION['form_key'];
            $this->formKey = $this->generateKey();
            $_SESSION['form_key'] = $this->formKey;
        }
    }

    //Function to generate the form key
    private function generateKey() {
        //Get the IP-address of the user
        $ip = $_SERVER['REMOTE_ADDR'];

        //We use mt_rand() instead of rand() because it is better for generating random numbers.
        //We use 'true' to get a longer string.
        $uniqid = uniqid(mt_rand(), true);

        //Return the hash
        return md5($ip . $uniqid);
    }

    //Function to output the form key
    public function outputKey() {
        return $this->formKey;
    }

    //Function that validated the form key POST data
    public function validate() {
        //We use the old formKey and not the new generated version
        if($_POST['form_key'] == $this->old_formKey) {
            //The key is valid, return true.
            return true;
        }
        else {
            //The key is invalid, return false.
            return false;
        }
    }
}

Edit: changed back to single key. Just call outputkey() to when needed. Don't create more than one instance of this class.

Answer 2

You could put this into a class, but this is needless complexity. Simple security systems are best because they are easier to audit.

//Put this in a header file
session_start();
if(!$_SESSION['xsrf_token']){
     //Not the best but this should be enough entropy
     $_SESSION['xsrf_token']=uniqid(mt_rand(),true);
}    
//$_REQUEST is used because you might need it for a GET or POST request. 
function validate_xsrf(){
   return $_SESSOIN['xsrf_token']==$_REQUEST['xsrf_token'] && $_SESSOIN['xsrf_token'];
}
//End of header file. 

The extra && $_SESSOIN['xsrf_token'] makes sure this variable is populated. Its there to make sure the implementation fails securely. (Like if you forgot the header file doah! ;)

This following html/php goes in any file you want to protect from XSRF, make sure you have the code above in a header file.

if(validate_xsrf()){
   //do somthing with $_POST
}

This is all you need to print out the form, again make sure you call session_start(); before you do anything, it doesn't matter if you call it multiple times.

<input type="hidden" name="xsrf_token" id="form_key" value="<?=$_SESSOIN['xsrf_token']?>" />