Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSRF method uses existing form value over correct session csrf value. #6

Open
edelooff opened this issue Oct 9, 2013 · 1 comment

Comments

@edelooff
Copy link

edelooff commented Oct 9, 2013

I have a schema that defines a single non-empty text field and sets the controls that allow additional fields and filters them. The schema looks like this:

class Message(Schema):
  """Message reply."""
  allow_extra_fields = True
  filter_extra_fields = True

  message = validators.UnicodeString(not_empty=True, max=500)

It is then processed in the following view code:

@view_config(request_method='POST', renderer='message.mak')
def claim_update(context, request):
  thread = context.data
  form = pyramid_simpleform.Form(
      request, schema=schemas.Message)
  if form.validate():
    # ... persist and redirect
    return exc.HTTPSeeOther('/message')
  return {'thread': thread, 'form': FormRenderer(form)}

The form renderer adds a csrf token and then the form is submitted by the user. The view code runs again and after validating (and stripping of attributes not in my schema) the form.data dictionary looks like this:

{'_csrf': u'6921efe037911dfe28991802462034c227173a06',
 'message': u'',
 'submit': u'Add message'}

The attributes that should have been stripped out have not been.

Performing a standalone to_python on my Message schema does work as expected (only the message gets returned, and when empty, Invalid is raised.

This causes me trouble because I'm resetting the csrf token on every successful post, but the old csrf value keeps coming back.

@edelooff
Copy link
Author

edelooff commented Oct 9, 2013

Looking over the code, the problem seems to occur at line 191 in init.py:

self.data.update(params)

        if self.schema:
            try:
                self.data = self.schema.to_python(decoded, self.state)
            except Invalid, e:
                self.errors = e.unpack_errors(self.variable_decode,
                                              self.dict_char,
                                              self.list_char)

Just before the validation code, all form values are added to the data attribute.

Thinking about it though this makes sense because otherwise one value being invalid would cause all form data being removed.

This does mean that the CSRF checking code should not get the value, but pop it from the MultiDict. This way, it will not be present when the form gets rendered and the csrf placing code will do its intended action.

Although, the better solution would probably be to correc the csrf method to not render through the hidden method which does a lookup of the _csrf key through value where the correct CSRF key becomes a default and not the override value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant