[pLog-svn] r7015 - plog/branches/lifetype-1.2/js/ui

[Reto Hugi]

Hi Jon

I didn't actually test the code, but just read your svn commits, so
maybe I've overlooked or misinterpreted stuff...

> I think I'll leave it as a plugin, but make it installed by default; it's 
> a nice code separation, keeping that code simple.

At least it was one quick way to ad the protection through the plugin
event. But parsing the the rendered page is expensive and thus it's
probably not an option for high traffic sites. Nevertheless I think if
we implement CSRF protection like that, it's a must that the plugin
comes pre-installed.

...and excluding actions that don't need protection would of course be
safer, but you knew that.

> 1. Plugins.  Not sure how to fix this unless we protect all plugin input, 
> using the configuration key array.  Or if I switch to be a whitelist, 
> rather than a blacklist, but that makes the URL parsing harder

Good point. That's probably the limitation of adding the protection on
the rendered HTML. Else you could have provided an API for plugin authors.

> 2. Remote URIs shouldn't ever have a CSRF token added.  I've thought about 
> this a little, and I think it can be done with regexps, all fully 
> qualified URIs need to be checked to see if they are really local or not. 
> And then all relative URLs should be protected.  I don't think we need to 
> worry about forms that post externally.  That doesn't exist, right?

Correct, I think we don't have to worry about external URLs.

Please have a look at the /class/data/nonce.class.php (not the
randomizer - yours is better) and at the
class/action/admin/adminaction.class.php (Line 174).
The implementation of the csrf-branche defers to yours in that the nonce
changes with every request which effectively protects from
replay-attacks. Of course this would stop support for multiple tabs in
one browser.

Overall I think your approach makes a good trade off between effort and
protection. I'll definitely upgrade but probably not the next 2 months...

all the best,
Reto