AddForm/EditForm request method not enforced to be POST (leading to CSRF)

Bug #805794 reported by Tommy Yu
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
z3c.form
Confirmed
Undecided
Unassigned

Bug Description

While the request method of the form is defined, it is not checked during data extraction for Add/Edit forms, such that data that is meant to be submitted in a POST request can also be done with a GET. This opens casual implementation/extension of AddForm/EditForm to CSRF (Cross Site Request Forgery), where the attacker can trick its victim to visit specially crafted links to modify data on target system using the victim's elevated permissions.

A possible fix can be adding a check to the extractData method, have it validate whether the request.method is equal to the form instance's method, and/or have a new attribute that specifies which request methods are allowed, such as query forms where submitted data do not cause side effects.

Revision history for this message
Hanno Schlichting (hannosch) wrote :

Adding generic CSRF support to z3c.form has been discussed on the mailing list. At this point you need to take care of it manually in your own forms. The form framework doesn't promise you any protection, so it's a missing feature but not a bug from our perspective.

At some point the form framework might offer integrated support for it. You can read up on the discussion at https://mail.zope.org/pipermail/zope-dev/2011-April/042760.html

security vulnerability: yes → no
visibility: private → public
Changed in z3c.form:
status: New → Confirmed
Revision history for this message
Domen Kožar (ielectric+) wrote :

Is there any public code related to this issue?

Revision history for this message
Tres Seaver (tseaver) wrote :

> Is there any public code related to this issue?

No.

Revision history for this message
Domen Kožar (ielectric+) wrote :

Just a stream of thoughts.

a) CSRF protection must be enabled by default (by convention "by default deny")

It is going to break JavaScript forms, but I would go for security regression.

b) PostOnly should be replaced by a list of methods that form handler allows

Probably "restrict_http_methods" parameter to ButtonHandler

c) plone.protect should also check for X-CSRF-Protect header for validation

This would ease JavaScript submission of forms. Developing a plugin for jQuery that would extract the token and set the header is then trivial.

e) protect decorator is an ugly CPython specific hack, I would encourage for z3c.form to use CSRF view multiadapter as authenticator

Revision history for this message
Stephan Richter (srichter) wrote :

Just to explicitly spell it out: If anyone provides a patch, I will apply it and make a release. The requirements in general are:

  1. Do not add excessive dependencies, rather provide hooks with simple (but effective) default implementations.

  2. JS Form submission must work.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.