ExtJS lack of html encoding

[jank]

We've been over this topic many times.

Anyway, I fall squarely in the "ext should encode by default" camp, and I'll list my argument:

1. You shouldn't store html-encoded data in your server, because you may need to present it in different contexts than a web context (I write a web app that has a windows native counterpart displaying the same data).
2. If you're using an RPC-like API to access the server (e.g. Ext.Direct, or JSON-RPC in my case), you shouldn't encode on the server, because you may have non-web clients accessing the same server API.
3. Encoding for output is a view layer responsibility, in an Ext app the view layer rests fully on the client.
4. Despite what I've heard many times, encoding on the client is neither slow nor insecure.
5. If you're encoding on the client, Ext forces you to write a lot of boilerplate code to do that. Boilerplate code is a potential for bugs. Ext's design in this respect encourages bugs, and even worse, they are bugs that potentially expose XSS issues.

+1!

[Uberdude]

+2!

In order to overcome Ext's poor behaviour around html encoding, we wrote a load of overrides so that you can fix it, for example:

/**
 * Html escape values for rendering, converting the empty value to non-breaking space to prevent rendering problems (e.g. in ComboBox dropdown)
 */
escapeRenderer: function(value) {
    if (value === undefined || value === null) {
        value = '';
    }
    else {
        value = String(value);
    }

    if (!value) {
        return '\u00A0';
    }

    return Ext.util.Format.htmlEncode(value);
},

/**
 * Html escape for rendering as escapeRenderer does, but also convert new line characters to <br/> tags.
 */
messageRenderer: function(value) {
    return Ext.util.Format.nl2br(Acme.escapeRenderer(value));
},

/**
 * If you put "foo <bar> baz" into a combobox's display field then it will correctly display as "foo <bar> baz"
 * in the text box portion when selected, but in the dropdown appears as "foo baz" as here the data is considered as html not text.
 * Fix this inconsistency by html escaping display value in dropdown.
 * Note that the implementation of this only applies where a custom tpl config setting is not provided, if you provide one
 * you are responsible for escaping (and indeed might not want to so as to display custom html rendering).
 */
escapeComboBoxDropdowns: function() {
    if (!escapeComboBoxDropdownsCalled) {
        // We add in our custom function BEFORE the normal initList.
        Ext.form.ComboBox.prototype.initList = function() {
            if (!this.tpl) {
                this.tpl = new Ext.XTemplate('<tpl for="."><div class="x-combo-list-item">{' + this.displayField + ':this.messageRenderer}</div></tpl>');
                this.tpl.messageRenderer = Acme.messageRenderer;
            }
        }.createSequence(Ext.form.ComboBox.prototype.initList);
        escapeComboBoxDropdownsCalled = true;
    }
},

You then specify which patches you want in your project's main.js (or whatever you call it):

Acme.ext.Patch.escapeComboBoxDropdowns();
Acme.ext.Patch.escapeGrids();
Acme.ext.Patch.escapeTreeNodes();
...

[ayqazi]

+2!

In order to overcome Ext's poor behaviour around html encoding, we wrote a load of overrides so that you can fix it, for example:

+infinity

You, my friend, are Uber. I've been reading the docs for ages trying to figure out how to do that.....

[ayqazi]

Edit: unfortunately, that does not seem to be built for ExtJS 4.0.2a, which is what I'm using. (I tried doing a recursive search for initList in the whole ExtJS src directory, but couldn't find that method anywhere, so it will never work for me as there is no method for it to hook into).

[Uberdude]

Yes, that patch was written for Ext 2.2, and I've used it on Ext 3.3.1 as well, but I've not used Ext 4. As it's messing around with internal details it's liable to break between versions.

[robert.peszek]

+3

Here is a good example: go to Ext 4 examples Rest Proxy Grid example:

http://dev.sencha.com/deploy/ext-4.0...l/restful.html

This is a grid you can edit. In the first row re-type first entry (which happens to be email address) as beginning of HTML comment:

 <!--

Grid stops showing any records!

[joeri]

That's a pretty damning bug you've found there. It demonstrates the exact problem that not having sane defaults in the framework makes encoding bugs unavoidable.

Update:
I came across this blog post outlining the reasoning behind html-encoding at the last possible moment: http://www.jardinesoftware.net/2011/...ould-i-encode/

[edspencer]

Totally agreed that we need to provide a solution for this. It's on the feature pile now and I'll bring it up internally at our next planning meeting

[Uberdude]

Thanks, glad it's on the radar at Ext HQ.

[everbrightlee]

Hi all,

I'm new to ExtJS, but am very impressed by the amazing capabilities provided this framework. I'm considering if this could be a pontential platform for some applications in my organization. However, I need some assurance to my management that security concerns are addressed by the framework as well, before they will allow my team to propose solutions based on this.

Have seen the following long discussion on this issue:-
http://www.sencha.com/forum/showthre...warning/page15

My personal take on this: there are responsbilities that clearly belong to server-side technologies, like data validation, SQL injection protection, etc. But client-side libraries and frameworks (of which extJS is one) have their responsibilities as well in the security context. The main danger here is that allowing malicious code through such holes may not result in successful attacks against the application itself (which should be protected by server-end mechanisms), but can cause data from the application to be sent to other "not-so-friendly" systems. I think the major holes to look at to address this are as follows:

1. Output-encoding/escaping
This is clearly in the domain of the client-side library, since it concerns the presentation on the client-end. I submit that not all of these can be fixed by proper server-end protection, because responses from the server end can be tapped and hacked as well to pass malicious code to the client end. I think that encoding should be turned on by default (agree to disagree!), but it should be able to optionally turn it off if there are scenarios where this really needs to be done, and the application is prepared to accept or mitigate the risk involved in doing so.

2. Validation of JSON/XML responses
These are the responses from server end where it is impossible to perform 1) above by default, since you really need the JSON format to be raw. What I personally think should be the approach is to provide for the same encoding/escaping mechanism for 1) for the data fields instead of the JSON response itself.

What of the performance impact, e.g. to data intensive components like Grid?
The key question to ask is this: what is the price of security? Performance may be a reasonable price to pay in some security-conscious or sensitive contexts. So it does help a lot if the framework provides help for such context out-of-the-box, without needing to resort to some hacks to the extensible framework.

Am especially glad to see Ed's response to highlight this issue to the Ext JS team. Hope to see something official soon that will tip the scales in favour of ExtJS in my context

Thanks!