[Security] XSS attacks for Extjs Applications - critical warning

[trbs]

I'm -0 on 'fixing' this. Everybody should note that the server should always (and i really mean always) clean, validate and shrub input. (Always and Everytime) If escaping would be the default then there will be some people screaming about that Besides a framework should escape everything or nothing (and let the developer escape it himself) one decision and stick with that which good documentation

But i'm +1 on creating a special section in the Documentation and Learn/Wiki section of the website explaining these kind of web development security issues/pointers.

[haibijon]

I know my previous posts may seem a bit brash, I just don't think this thread is about a security issue. Don't get me wrong, XSS are a very real problem, I just don't think client-side default escaping is the answer, and that's really what this thread is about (since an XSS using the grid previously described would require served data to be unescaped).

But i'm +1 on creating a special section in the Documentation and Learn/Wiki section of the website explaining these kind of web development security issues/pointers.

I also +1 this. I think that it would also be great to highlight the built-in escaping methods that ExtJS provides, and possibly any security extensions that may developed in the future.

Maybe it should be a wiki page?

[Hani]

The fix posted by Jack in this thread doesnt actually work, the right fix (or at least one that works for me) is to use a subclass of ColumnModel to handle it. This works in both 1.x and 2.0:

Ext.grid.SecureColumnModel = Ext.extend(Ext.grid.ColumnModel, {
     getRenderer : function(col){
         var c = this.config[col];
         if(!c.secureRenderer){
             var fn = c.renderer;
             if(!fn){
                fn = Ext.grid.ColumnModel.defaultRenderer;
             }
             c.secureRenderer = function(val, p){
               var rendered = fn.apply(window, arguments);
               return val && val != '' && typeof val != 'boolean' ? Ext.util.Format.htmlEncode(rendered) : rendered;
             }
        }
        return c.secureRenderer;
     }
})

[jaanus]

Validating and escaping input on the server side is a good practice. Combine it with the design where you have only one gateway for all the data sent by the client and you will have a very secure and fast application.

[bidochko]

Personally I do not think an ExtJS grid component should be responsible for any type of escaping unless application functionality requires it. For example, server might send a valid IMG tag which should be rendered like an image without any escaping, right?
Of course in some cases escaping is necessary and you have plenty of methods how to do it: Ext.util.Format.stripTags on a client side or doing some server side escaping.

~ Andrew Bidochko
http://mashuptechnologies.com/

[ofir]

Thanks for the solution Hani, I think many people in this thread haven't understood the problem quite well.

This issue doesn't even have to do with server-side escaping at all, just try entering this in the editable grid from the example: User <user@email.com>.
The email address magically dissappears because anything inside the <> characters is treated as html, which it shouldn't.

I hope this gets fixed in the next ExtJS version!

[Hani]

Actually, despite the protestations of some, I'm glad to say that 2.0.1 fixes this issue and adds an autoEncode option to editorgrid, which does all the encoding needed. I'm glad someone saw reason, though I am curious as to what caused this enhancements to happen, despite being told in this thread that it doesnt make sense to add such a config option

[jack.slocum]

Since it's done on input (not with a renderer), it is a little different. I think it makes for a good compromise, especially since the developer has the option of turning it on/off.

[ofir]

The autoEncode option works as long as you don't send your data to the server on the afteredit event, which I'm guessing it is triggered before the encoding.

[kmbarr]

I don't always want to deal with html-escaped text in my database for sorting reasons, reporting, etc. What am I missing, isn't this as simple as adding a renderer to any vulnerable column, like:

{header:'Venue Name',dataIndex:'venue_name',renderer:Ext.util.Format.htmlEncode},

I think this is a common-enough requirement, that I'd like to see it simplified by adding a simple htmlEncode (or maybe autoEncode) boolean to the ColumnModel, so the result would look like:

{header:'Venue Name',dataIndex:'venue_name',htmlEncode:true}