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

Bug #805794 reported by Tommy Yu on 2011-07-05
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
z3c.form
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.

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
Domen Kožar (ielectric+) wrote :

Is there any public code related to this issue?

Tres Seaver (tseaver) wrote :

> Is there any public code related to this issue?

No.

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

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  Edit
Everyone can see this information.

Other bug subscribers