4
\$\begingroup\$

I googled around for secure random number generation and random string generation, combining them with some user data.

Is this good or am I totally off-base? I don't know much about cryptography but I do not see many alternatives, other than some bad code that gets copied around a lot with mt_rand and uniqid.

//csrf tokens
    public function csrf_token($regen = false)
    {
        if($regen === true) {
            //we need to give the user a token
            if(isset($_SESSION["__csrf_token"])) {
                unset($_SESSION["__csrf_token"]);
            }
            $max = mt_rand(0, mt_getrandmax());
            $rand_num = floor($max*(hexdec(bin2hex(openssl_random_pseudo_bytes(4)))/0xffffffff));
            $rand_string = "";
            for($i=0; $i < 11; $i++) {
                $x = mt_rand(0, 2);
                switch($x) {
                    case 0: $rand_string.= chr(mt_rand(97,122));break;
                    case 1: $rand_string.= chr(mt_rand(65,90));break;
                    case 2: $rand_string.= chr(mt_rand(48,57));break;
                }
            }

            $_SESSION["__csrf_token"] = hash('whirlpool', $rand_num . $this->username . $rand_string . $this->hash_pw);
            $this->csrf_token = $_SESSION["__csrf_token"];
            return $this->csrf_token;
        }else{
            //the user already has a token
            return $this->csrf_token;
        }
    }
\$\endgroup\$
2
  • 1
    \$\begingroup\$ To me, it looks like it's doing too much. I'd just generate 16 random bytes, convert them to hex, and be done with it. \$\endgroup\$
    – icktoofay
    Apr 13, 2014 at 1:06
  • \$\begingroup\$ thanks icktoofay. you're probably right. i will consider this when refactoring the code. thank you for your time. \$\endgroup\$
    – r3wt
    Apr 13, 2014 at 1:34

1 Answer 1

3
\$\begingroup\$

If CSRF stands for Cross Site Request Forgery, then it's hard to imagine why I should help.

In any case, simply doing a cryptographic Whirlpool hash of a user-supplied string with a random seed value should be sufficiently random for most every purpose. The rest is just obfuscation and doesn't add to security.

\$\endgroup\$
5
  • \$\begingroup\$ I appreciate your reply. i am not the sharpest knife in the drawer and it is hard for me to decide how much is good enough. so what i infer from your post, i should just hash the hash_pw(user supplied string) with a random number(Random seed), or should i hash it with the random string as well. \$\endgroup\$
    – r3wt
    Apr 13, 2014 at 1:33
  • \$\begingroup\$ Related meta post on malicious code. \$\endgroup\$
    – syb0rg
    Apr 13, 2014 at 2:36
  • \$\begingroup\$ @syb0rg how is that related? \$\endgroup\$
    – r3wt
    Apr 13, 2014 at 6:09
  • \$\begingroup\$ @r3wt It was more related to Edward's comment about why he found it hard to help. \$\endgroup\$
    – syb0rg
    Apr 13, 2014 at 16:18
  • 1
    \$\begingroup\$ i'm lost.. what about my code is malicious? \$\endgroup\$
    – r3wt
    Apr 13, 2014 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.