plone / plone.protect

HTTP protection utilities for the Plone CMS
https://pypi.org/project/plone.protect/
7 stars 8 forks source link

CSRF protection failure on object with many annotations #88

Closed reinhardt closed 5 years ago

reinhardt commented 5 years ago

I have a bit of an odd problem where an object that has a lot of annotations and is locked for edit via plone.locking for the first time is causing a CSRF protection failure when there is no authenticator passed. This is on a GET of the edit form, so no actual editing is happening yet.

To reproduce:

You are asked to confirm the operation. In the log you see that the transaction has been cancelled due to no CSRF protection.

In my case I'm running quaive where there used to be some code that added a lot of custom annotations. But it doesn't seem to matter where they come from. I was also able to reproduce by just slapping 20 random keys with some string and datetime values on the object. It seems to be the sheer size that triggers the issue.

My analysis so far suggests that plone.locking creates an annotation when locking an object for the first time. It marks the annotations as safe to write, so it should not trigger a CSRF protection failure. And usually it doesn't. However, safeWrite only marks the OOBTree that holds the annotations itself. When the tree has reached a certain size it's no longer the OOBTree itself but an OOBucket that gets registered for writing in the transaction. plone.protect then looks up the oid of the OOBucket in the safe_oids but there's only the OOBTree's oid, so it bails out. (around here)

It looks like one way or another the OOBucket needs to be marked as safe to write as well. As it's an implementation detail, plone.locking should not have to deal with it. So I guess it's down to plone.protect figure out whether an OOBucket is safe to write even though it's not marked as such.

I'll see if I can come up with a failing test or even a fix.