PHP CSRF eliminator

Question

There are three functions involved. The first Init is run before the content is parsed. The second is parseContent where the content is generated. The third is displayPage that renders the page and HTML for the user.

They are within an OOP class that uses template files which have #KEYWORDS# that are dynamically replaced. session_start() is also in effect

Init()

$this->content = false;    
$this->inputVerified = false;

if ( $this->input['token'] && $_SESSION['TOKEN'] && $this->input['token'] === $_SESSION['TOKEN'] )
{
    $this->inputVerified = true;

    unset( $_SESSION['TOKEN'] );
}

parseContent()

// content here

if ( $form_submission )
{      
    if ( $this->inputVerified )
    {
        // CSRF PROOF?
    }
}

$this->content = 'DYNAMIC TEMPLATE CONTENT';

displayPage()

if ( strpos( $this->content, '#TOKEN#' ) )
{
    $main = hash_hmac( 'sha512', mt_rand(), mt_rand() );

    $_SESSION['TOKEN'] = $main;

    $this->content = str_replace( '#TOKEN#', $main, $this->content );
}

What I like about this is I only had to do the following to implement it:

Include a hidden token input field with the value of #TOKEN# on any sensitive form
- displayPage() will detect and replace this (with questionable efficiency)
Check for $this->inputVerified on any page that will process a sensitive form
- Init() will detect token input and if it matches its $_SESSION counterpart will set $this->inputVerified to true only on the first valid submission, false if refreshed or otherwise.

Is it enough?

lol. In my experience it's rarely enough. Welcome to Code Review. I hope you get a nice review. — RubberDuck
– RubberDuck, Commented Mar 29, 2015 at 11:42
Me too, but an honest, no-holds-barred review helps everyone. Always appreciate a warm welcome! Definitely could use proper hash verification for the final $_SESSION - $token comparison. Maybe set the HTML token to the hash data and $_SESSION to the final hash salted with the IP, that way the submitted token salted with the current IP must be equal to the original token salted with the original IP? — John
– John, Commented Mar 29, 2015 at 12:34
I really couldn't tell you. I saw your question in the First Post queue. I don't PHP myself. — RubberDuck
– RubberDuck, Commented Mar 29, 2015 at 12:36

tim · Accepted Answer · 2015-03-29 13:21:20Z

Now the question that has me on the edge of my seat, is it enough?

To protect against CSRF? Yes, using an anti-csrf token is the recommended way to protect against CSRF, and you seem to be doing it correctly. Although you should note that just one XSS vulnerability in your website would make all CSRF protection useless, see this article I just wrote about bypassing CSRF Protection via XSS.

Although I'm assuming that either your instances of this object are not reused, or that you set $this->inputVerified to false/call Init when appropriate?

Misc

I'm not sure how much I like the idea of using placeholder strings, but as you are doing it: I think it would be nice to add an additional check: if there is a form, but no #TOKEN# in the content, raise a warning. I know that not all forms need CSRF protection, but it doesn't hurt either. If you only want protection on some forms, you could pass a boolean to displayPage that can optionally disable this check.
your structure doesn't seem very clear to me. I would probably create a CSRFProtection class, which generates tokens, compares them, sets them to a session, etc.
$main seems like an odd variable name. $anti_csrf_token seems more appropriate.
true only on the first valid submission, false if refreshed: Is this actually something that's needed? This doesn't seem to add any security, and it might be quite annoying for users. Also, there is already a mechanism to avoid simple form resubmits. Mixing resubmit-detection with CSRF protection doesn't seem ideal to me (mainly because it will be hard to report to the user what actually went wrong).

Stack Exchange Network

PHP CSRF eliminator

1 Answer 1

You must log in to answer this question.

Hot Network Questions

PHP CSRF eliminator

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions