PDA

View Full Version : [CLOSED] com.extjs...widget.grid.GridView.getRenderedValue does not escape <



Curt Arnold
22 Apr 2009, 7:56 PM
If a value bound to a grid cell contains is contained in <>, the contents between the angles will not be displayed unless there is an explicit GridCellRenderer which properly escapes the angles as &lt; and &gt;.

The issue in at line 955 in GridView.java where val.toString() is returned without proper escaping.

I assume (but can't confirm at the moment) that this would be visible if some stock data were changed in TestData to be enclosed in <>. Surprised that this hadn't been reported before, but I did search.

The workaround is to provide a custom GridCellRenderer that properly escapes its output. Without escaping, it is possible that some malicious user could perform the HTML equivalent of SQL injection.

sven
22 Apr 2009, 10:05 PM
It is up to you to return the data in the correct format.



The workaround is to provide a custom GridCellRenderer that properly escapes its output. Without escaping, it is possible that some malicious user could perform the HTML equivalent of SQL injection.

This should also be filtered serverside.

Curt Arnold
23 Apr 2009, 5:59 AM
Strongly disagree here. The model data is independent of the context and should not be aware of where it might be rendered. Also to make things safe without fixing the default renderer, you would have to ensure that the toString() return value of any object that you had in the model would not contain <'s and that all things adding content to the model where aware that it had to be HTML-escaped.

You can observe the issue using the editable grid example (http://extjs.com/examples/grid/editable.html) without modification. Just modify "Bloodroot" to "B<loodro>ot" or "Cowslip" to "Cow<input type="button">slip". You got to admit that users should not be able to hack the UI like that. Somebody needs to be responsible and the right place is the renderer since it should be the thing that is responsible for formatting the data for presentation.

Unfortunately, it looks like ExtJS has the same issue since you can do the same with its grid example: http://extjs.com/deploy/dev/examples/grid/edit-grid.html

sven
23 Apr 2009, 6:00 AM
It is not really an issue. It is up to you to allow and format the data as you want it.

Curt Arnold
23 Apr 2009, 10:19 AM
Did some more seaching and it appears has been frequently brought up as a weakness in ExtJS, but the ExtJS team dismisses it as not a concern.

http://extjs.com/forum/showthread.php?t=13913
http://www.akbkhome.com/blog.php/View/154/Extjs_gtkDjs_and_other_updates.html
http://cwe.mitre.org/top25/index.html#CWE-116

While I suggested examples with an malicious intent, I ran across the issue like one of the other observers totally valid content that was surrounded in <> that disappeared. My was generated from an value object's toString() method that appeared in the model data. No reason that it should need to be aware of the presentation layer.

Basically, my takeaway is to not trust the default renderers in Ext GWT and always provide a renderer. It would be great if Ext GWT 2 would see the major version change as an opportunity to fix the default renderers to properly escape data. For the edge cases where the model data is supposed to be interpreted as markup, a user can provider renderers that don't escape the model data.