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 formdisplayPage()
will detect and replace this (with questionable efficiency)
- Check for
$this->inputVerified
on any page that will process a sensitive formInit()
will detecttoken
input and if it matches its$_SESSION
counterpart will set$this->inputVerified
totrue
only on the first valid submission,false
if refreshed or otherwise.
Is it enough?
1 Answer 1
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 todisplayPage
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).
Explore related questions
See similar questions with these tags.
$_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? \$\endgroup\$