csrf_protect.php. A PHP class for applying CSRF protection to existing PHP applications, using output buffering to rewrite any POST forms on a page. Heavily inspired by Django’s CSRF middleware. Tell me if you spot any bugs!
Sign in with 
OpenID
I know nothing about PHP and little about security, but a few rough comments:
Whitespace around the '=' in method="POST" is legitimate and not uncommon, so it'd be nice if the regexp matched that.
If the page contains a POST form to a remote untrusted page (e.g. a search engine form or something), the _csrf_protect_token will be sent to that page, and an attacker controlling that page can use the same token for CSRF on the original site. I guess it'd be safer if the token was only sent with local form requests (e.g. only when 'action' is a relative URL).
With what PHP apparently calls transparent SIDs, where the session ID is in a query parameter rather than a cookie, it's easy for an attacker to find the session ID (e.g. from HTTP Referer, or copied-and-pasted URLs). Since generate_token depends on nothing other than the session ID, the attacker can then easily calculate the token. I guess it'd be safer if generate_token depended on a site-specific secret value concatenated with the session ID, so an attacker can't calculate it without knowing the secret value.
Philip Taylor - 24th September 2008 16:16 - #
Philip: good points all. I designed the class to be sub-classable for customisation, so the fix to your SID query parameter issue would be to over-ride generate_token() for your specific site. Personally I think forged CSRF tokens are the least of your problems if you're handing session IDs around in your URLs :)
The fixes to your other two issues both require slightly more intelligent <form> tag parsing - I'll have a think about the best way to do that such that people subclassing CsrfProtect can customise it to fit their specific needs.
headers_list()requires PHP >= 5. To avoid any confusion, you might want to ditch the deprecated 'var' property declaration.isset()checks on$_SERVER['empty($_SERVER['HTTP_X_REQUESTED_WITH']and$_POST[$this->csrf_form_field]would be nice to avoid PHP warnings.Michel de Lange - 24th September 2008 16:50 - #
Simon,
This is a good offering, but I disagree with your approach of suggesting override to make generate_token actually secure. Many people will directly use the class and not realize there is this threat vector.
Forgetting the SID-in-URL problem (which you can't save them from), why not require a salt to be passed into the CSRFProtect constructor? Secure by default, no?
Jeremy Dunck - 24th September 2008 17:02 - #
Are spaces around '=' actually legitimate?
Although not possible with the class as it is.
Is passing "$form_token != $csrf_token" not a great way to find out the CSRF token if that error string is printed?
Attacker posts with _csrf_protect_token=junk and then uses the error string to find the legit token?
Hmm, just thought about this. No, they couldn't. Doh
arty - this page suggests that white space is allowed around the equals sign between attribute name and value although there is no mention (either for or against in the HTML 4.01 spec). Either way it'd probably make sense for the regular expression to take account of this possibility.
Dan - the "
$form_token != $csrf_token" is just testing code which should probably be removed. It isn't printed in the error method.Simon - it looks to me like $csrf_form_field is something people might want to change. I'd suggest marking it protected (
protected $csrf_form_field;) and then either optionally setting through the constructor or through a set method.Ed, thanks, I didn't find this document on w3 site