11

I am wondering if this is a secure way to set a token, unless there actually is a token generated, I generate one, and use it throughout the applications and those forms. One token per session?

if (!isset($_SESSION['token'])) {
    $data['token'] = uniqid(rand(), true);
    session_regenerate_id();
    $_SESSION['token'] = $data['token'];
}

Would it be necessary to clear out the token on a submitted form? or just stay with it, even though I submitted a form?

user1909426
  • 1,658
  • 8
  • 20
user1831020
  • 217
  • 3
  • 13
  • 2
    Possible duplicate: [CSRF protection: do we have to generate a token for every form?](http://stackoverflow.com/q/8655817/53114) – Gumbo Dec 30 '12 at 12:30
  • 1
    That code by itself is not sufficient. That token used as a salt alongside a unique identifier for a (group of) form(s), mitigates damage on leaks and also keeps usability for users. You also need to mitigate issues regarding expiration. If your tokens are forever useable. That's a design flaw right there regarding CSRF. – Khez Jan 04 '13 at 15:54

6 Answers6

11

If you don't know these links, this should help you understand some scenarios and specifically this will tell you the DOs and DONT's. Hope it helps.

palako
  • 3,342
  • 2
  • 23
  • 33
  • Just +1 for linking that material. Even I normally dislike link only answers, this is to a wiki where more than one person cares about the topic. – hakre Jan 06 '13 at 01:00
10

Personally I would generate a new token for every form I want to display. If you do it this way, someone just needs a session cookie to read your token and use it as long as the session stays active.

In my applications I generate a token for each form display like this:

<?php
$token = uniqid(rand(), true);
$_SESSION['csrf_tokens'][$token] = true;

HTML

<form>
    <input type="hidden" name="token" value="<?php echo $token ?>" />
</form>

On form validation I check for that token like this:

if (isset($_SESSION['csrf_tokens'][$token]) && $_SESSION['csrf_tokens'][$token] === true) {
    unset($_SESSION['csrf_tokens'][$token]);
    // additional code here
}
Benjamin Paap
  • 2,744
  • 2
  • 21
  • 33
2

Rather than use per-session token i would prefer per-form/url token for additional security some might argue that per-request token is most secured but affects usability.

I also think its better to separate your session storage from your token storage and use something like Memcache. This is better when you need speed using multiple application servers etc. I also prefer it because i can add a custom expiration to the token without having to affect the whole session

Here is a typical example

HTML

<form method="POST" action="#">
    IP:<input type="text" name="IP" /> <input type="hidden" name="token"
        value="<?php echo Token::_instance()->generate(); ?>" /> <input
        type="Submit" value="Login" />
</form>

Processing

$id = "id44499900";
Token::_instance()->initialise($id); // initialise with session ID , user ID or IP

try {

    Token::_instance()->authenticate();
    // Process your form
} catch ( TokenException $e ) {
    http_response_code(401); // send HTTP Error 401 Unauthorized
    die(sprintf("<h1>%s</h1><i>Thief Thief Thief</i>", $e->getMessage()));
}

Class Used

class Token {
    private $db;
    private $id;
    private static $_instance;

    function __construct() {
        $this->db = new Memcache();
        $this->db->connect("localhost");
    }

    public static function _instance() {
        self::$_instance === null and self::$_instance = new Token();
        return self::$_instance;
    }

    public function initialise($id) {
        $this->id = $id;
    }

    public function authenticate(array $source = null, $key = "token") {
        $source = $source !== null ? $source : $_POST;

        if (empty($this->id)) {
            throw new TokenException("Token not Initialised");
        }

        if (! empty($source)) {
            if (! isset($source[$key]))
                throw new TokenException("Missing Token");
            if (! $this->get($this->id . $source[$key])) {
                throw new TokenException("Invalid Token");
            }
        }
    }

    public function get($key) {
        return $this->db->get($key);
    }

    public function remove($key) {
        return $this->db->delete($key);
    }

    public function generate($time = 120) {
        $key = hash("sha512", mt_rand(0, mt_getrandmax()));
        $this->db->set($this->id . $key, 1, 0, $time);
        return $key;
    }
}
class TokenException extends InvalidArgumentException {
}

Note : Note that the example might affect "Back" button or refresh because the token would be automatically deleted after 120 sec and this might affect user friendly capability

Baba
  • 94,024
  • 28
  • 166
  • 217
  • -1 Your approach is still vulnerable. You dont have "binding" to IP or session of original user. Attacker can get his token at realtime and use it for an attack. Also you generate token with hashing one number - that could be brute-forcable. – Zaffy Jan 02 '13 at 14:47
  • 1
    I don't agree with Zaffy. Binding sessions to IPs isn't a user-friendly way these days where lots of users use mobile devices with changing ips. Also, if someone is able to read someone's cookies or network stream, you've got worse issues than that. – Daniel M Jan 02 '13 at 16:24
  • @DanielM I wrote binding on IP `or` session but he has never assigned token to specific user with neither ip or session and so attacker dont need to know your cookies because he just can use his token as yours. – Zaffy Jan 02 '13 at 20:26
  • The asumbtion is that you would have a valid authentication process with session before you get to this stage .. never the less good constructive criticism .. updated class with use if `$id` .. thanks – Baba Jan 03 '13 at 09:15
  • 1) `TokenException` isn't `InvalidArgumentException`! 2) `DB` isn't `memcache` – Yang Jan 06 '13 at 23:44
2

I am wondering if this is a secure way to set a token

It depends on how secure your web app needs to be. This line is not cryptographically secure (As warned in PHP docs for uniqid() and rand()):

uniqid(rand(), true);

It may be feasible for an attacker to determine/brute force this if the time of token generation is known/determined and the rand() seed is known/determined. However, for your purposes it may be fine as it will still prevent CSRF attacks where the attacker has no knowledge of the token value.

One token per session?

Using one token per session may be fine for your purposes. However, be aware:

  1. If a session is n minutes long then an attacker has an n minute window to attempt to determine or obtain your token value and execute a CSRF attack. Whereas this risk is reduced when tokens are generated per form or when the token is regenerated periodically as they are not long lived enough.
  2. Using a single token per session exposes all of your application's functionality (that uses that token) to attack should an attacker determine/obtain the token. Whereas using a token per form restricts an attack to a single form.

Would it be necessarery to clear out the token on a submitted form? or just stay with it, even though i submitted a form?

It depends upon how high value a target your application is for attackers and the level of disruption an attack would cause you. Your existing measure makes it difficult to execute CSRF attacks but if it is high value and you have very determined attackers then you may want to reduce the risk of CSRF more by:

  1. Using cryptographically secure tokens to prevent risk of determining or brute forcing the token value.
  2. Regenerating the token periodically to reduce token lifespan, decreasing the attack window if the token is determined or obtained.
  3. Generating tokens per form to restrict attacks to a single form in the event that the token be determined or obtained.
1

Can you please refer following site, this may get some ideas.

1.) https://docs.djangoproject.com/en/dev/ref/contrib/csrf/

2.) http://blog.whitehatsec.com/tag/session-token/

Thanks for reply.

test
  • 49
  • 1
  • 6
1

I've already answered a similar question on a different forum: here. Hopefully that's helpful. It explains the basic process of the CSRF prevention and links to some code for a CSRF framework.

If you want higher security, change the token after each request for each session. If you want better usability, keep one token per session.

Scott Finlay
  • 86
  • 1
  • 5