pombreda / formalchemy

Automatically exported from code.google.com/p/formalchemy
MIT License
0 stars 0 forks source link

Field values should really be escaped. #156

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?

What version of the product are you using? On what operating system?

Please provide any additional information below.

Original issue reported on code.google.com by dair...@gmail.com on 24 Nov 2010 at 7:04

GoogleCodeExporter commented 9 years ago
Crap... I hit return in the report subject field, and the form submitted.
Sorry for the blank template above.

Anyway, what I want to say is that Field values should really be escaped
when rendering in readonly mode.  Without this, unless the developer goes 
through a number
of contortions, most apps built with any readonly string-valued data fields in 
their forms will
have XSS vulnerabilites.

To be concrete:
  - formalchemy.fields._htmlify should be fixed to wrap all values
    returns from _stringify in h.escape()
  - FieldRenderer.render_readonly also needs to be patched to
    h.escape() any unicode values returned.   (Or just omit
    that conditinal, and pass unicode values through
    .stringify_value(as_html=True) as it does with most other types of
    values.)

I realize the non-escaping is done on purpose and that applying an 
EscapingReadonlyRenderer can work around this.   But really, escaping
can be on by default.

When one wants to pass raw HTML as a value (as in the implementation
of the edit and delete buttons of the admin controller), just wrap the
HTML text in an h.literal() to prevent escaping.

(This ticket is somewhat a dup of 
http://code.google.com/p/formalchemy/issues/detail?id=119 , but I think it's 
important enough that it's worth revisiting.)

Jeff

Original comment by dair...@gmail.com on 24 Nov 2010 at 7:16