3
\$\begingroup\$

I'm currently working on CSRF prevention and I would love to hear your feedback for my code. I assume that there are no XSS vulnerabilities in order for the CSRF protection to work. Furthermore, I assume that all state-changing/critical actions are done via post-requests. One token is generated per session.

Here is the code:

<?php

/*
 * CSRF prevention module
 */

class CSRF {

    private static $key = "token";

    public static function init() {
        if (session_status() === PHP_SESSION_NONE)
            die("[CSRF::init] PHP session engine is not started");

        if (!self::has_tok())
            self::set_tok(self::gen_tok());
    }

    public static function inject_token() {
        if (!self::has_tok()) 
            die("[CSRF::secure_form] There is no token");

        echo "<input type='hidden' name='" . self::$key
           . "' value='" . self::tok() . "'>\n";
    }

    public static function valid_post() {
        $src = parse_url(self::source_origin(), PHP_URL_HOST);
        return $src === $_SERVER['SERVER_NAME']
            && isset($_POST[self::$key]) 
            && self::match_tok($_POST[self::$key]);
    }

    private static function gen_tok() { 
        return base64_encode(random_bytes(64)); 
    }

    private static function has_tok() {
        return isset($_SESSION[self::$key]);
    }

    private static function set_tok($tok) {
        $_SESSION[self::$key] = $tok;
    }

    private static function match_tok($tok) {
        return self::has_tok() && $tok === self::tok();
    }

    private static function tok() {
        return $_SESSION[self::$key];
    }

    private static function source_origin() {
        if (isset($_SERVER['HTTP_ORIGIN']))
            return $_SERVER['HTTP_ORIGIN'];
        if (isset($_SERVER['HTTP_REFERER']))
            return $_SERVER['HTTP_REFERER'];
        return "";
    }
}

?>

Usage example:

 <form action="foo.php" method="post">
     <?php CSRF::inject_token(); ?>
     ...
 </form>

And then in foo.php:

<?php
    ...
    if (!CSRF::valid_post())
        die("CSRF");
    ...
?>

Any suggestions for improvements? Did I miss anything? I know adding the tokens to forms could be automated with javascript (there is an OWASP CSRF Protection module), but I prefer to have a site that can run without javascript. I don't plan on having too many forms, so I think I can manage to not forget securing one.

I recently added checking the origin/referer after reading OWASP CSRF Prevention Cheat Sheet, but I'm not sure I did it properly.

EDIT: I just realized from the code example at OWASP that

(1) I should compare protocol and port, too, not just the host.

\$\endgroup\$
  • \$\begingroup\$ What I do is regenerate a CSRF token once it is used. Not once per session, Reason - It can be extracted by the user from inspect element and can be misused. \$\endgroup\$ – Rajendran Nadar Nov 4 '17 at 8:42
2
\$\begingroup\$

This seems quite reasonable as a baseline for CSRF protection. Critically, you must ensure that you do not set the Access-Control-Allow-Origin (CORS) header to a wildcard, otherwise an attacker can just perform an Ajax request to your site and gain access to the CSRF token.

A criticism of single global CSRF tokens is that that exposing the CSRF token allows re-use across different requests, which can be avoided. The upside of the single global token model is that it is very simple to implement and therefore has less chance of bugs. Additionally, it is unlikely that an attacker will gain read-access to only one page and not others, particularly as XSS would allow Ajax requests to other pages on the same origin (same domain).

If you wish to generate tokens that are locked to their initial purpose (i.e. a particular form target URI), a fairly sensible approach is to use HMAC-SHA256 to generate each token. Generate a global CSRF key as part of your session data, then for each token you need to generate compute a HMAC-SHA256 digest of the target URI using the CSRF key as the HMAC key, then inject that digest as the CSRF token in the form. For validation, take the request URI and hash it using the CSRF key, then compare that to the CSRF token provided as part of the request.

If you're building any kind of framework to handle MVC or MVVM then I would implement the CSRF protection as a by-default for all requests in the parent view or controller class, then have a way to opt-out of CSRF validation explicitly. CodeIgniter 3.x handles this quite nicely, so you should check out their code as an example.

One thing I would probably add is logging of cases where CSRF validation fails, although the way you do that is very much down to your platform.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Great answer, thanks. I'm not setting Access-Control-Allow-Origin: * anywhere, so I should be fine? I will stick to a per-session token for now, for simplicity. The error handling was just an example, and yes I should log those events. Additionally, I should probably check if the referer was just missing or if the token was bad/missing. \$\endgroup\$ – smoothware Dec 2 '17 at 2:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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