9 Sep 2008 12:47 AM #51
ApocalypseCow, thanks for your reply. I think it's all about data types. If we are talking about plain-text data, it's not "unsafe" until it's rendered as raw HTML. And the same data source could be used in other destinations as well.
I'd hate to see something like "<somevalue>foo &amp; bar</somevalue>" in an XML data stream if the actual payload was a plain-text "foo & bar".
This brings us to the very important point: the intended data type (plain-text, html text, etc) of each data item should be always very clear to the developer. (That's why I brought up the Apps Hungarian notation.)
And it's not only about server-client communication. Let's think of the editable grid. To me it seems awkward that the default configuration and the sample (http://extjs.com/deploy/dev/examples...edit-grid.html) work so that if a user enters "<b>foo" in a grid cell, he gets a bolded "foo". (Try it on the sample page!) This is clearly a bug. (Not an XSS bug.)
To fix this (little) bug, the htmlEncode renderer is needed anyway for the grid cells. Or something else more complicated.
I hope I could make my point a bit clearer now. This is a very good and very important conversation.
9 Sep 2008 1:17 AM #52
All of your explanations are not really problems until you save the xss string and send it also to the next user. So i dont understand all this crying...
Otherwise i could say, every website is XXS vulnerable, because with firebug i can change entire code of erver page at runtime. But, and thats the operative point, not for other users.
If I would really do a XSS attack, I have to change the source for other users...
With a grid itself, it is impossible...
Just my Opinion...
And i dont understand why it should be danger if my grid is displaying html...
9 Sep 2008 2:18 AM #53
Foggy, this particular Grid problem ("foo<b>") I described is not an XSS problem, as I said. But this example shows how the present Ext Grid forces the developer to *manually* take care of HTML encoding, when handling only plain-text data (since you can only write plain-text in an <input type="text"/>, as in the example).
The design of Grid and other components forces the developer to write htmlEncode() calls here and there, on the server or on the client, even if the application only handled plain-text data (which is a common case, I think). If the developer *forgets* html encoding somewhere, an XSS bug is introduced. I'd like to see a design where a safe encoding is *on* by default, and the user would have to turn it off. With this design we will see many XSS vulnerabilities in real applications.
How do people feel about my suggestions to fix the samples and API docs to emphasize the need to encode the (possibly user-supplied) data, even somewhere?
9 Sep 2008 3:13 AM #54
- Join Date
- Mar 2007
- Frederick MD, NYC, DC
- Vote Rating
I Don't see how it's Ext's responsibility to enforce security practices. It's up to the end developer. If the developer *forgets* to secure their application, it's the developer's responsibility, not that of Ext! Any good security minded developer could easily write low-level wrappers and overrides for the tools they want to use - and not 'manually' call encodes everywhere.
Do you want Ext to butter your bread too!?
9 Sep 2008 3:51 AM #55
If the developer *forgets* html encoding somewhere, an XSS bug is introduced.
I'd like to see a design where a safe encoding is *on* by default, and the user would have to turn it off.
With this design we will see many XSS vulnerabilities in real applications
How one of my previous speakers said: do this, for example in your jsonResponse class on Backend, strip every value you like to send via a json string. You could allow there your specific strings with HTML Code...
Or you save your string in your well defined format in the db. What to hell searchs a JS Script in my db column wich should be in plain text format? And even why i can save js (in <script> Tags) in a plain text column? There is absolutley no reason to not call stripTags on backend. In php for example, you can allow a bunch of tags and strip just the unallowed tags...
So, i hope you understand my "swiss-styled" englisch, greets
9 Sep 2008 4:39 AM #56
jgarcia, thanks for commenting again. I think we can agree that security should be all parties' responsibility. A framework can only do so much for the security, but I think that the approach taken by the Ext team is not optimal. Of course any good developer can write wrappers and overrides, but I believe many developers use Ext for some random small projects, only checking the samples and copy-pasting some code here and there - not getting really engaged with the Ext model. With the current samples and API docs, this introduces a potential for XSS holes, since the (not-so security oriented) developer may not have thought of this thing at all. Luckily this shortcoming can be easily fixed by commenting the sample code and API as I suggested.
One thing that could trick Ext newbies at first is that many components have attributes named 'text', Ext.Button for example. This is quite misleading, since they take *html* input, not plain-text, as one could expect. In contrast, eg. in jQuery it's quite clear what is going on in each one here: $(...).text('<b>foo') and $(...).html('<b>foo'). Of course, this all depends on what kind of background the developer has.
BTW, where do you think the encoding should happen in an editable grid? Should the server send the data encoded? If yes, no htmlEncode renderer for the Grid, right? What would be the right way to fix the 'foo<b>' bug I described earlier? Would the answer be something like "htmlDecode when a field edit starts and htmlEncode when it ends"? And then server-side decoding, when saving the data?
I'm still voting for client-side-encoding and non-encoded data transfer, because I don't see a clear answer to this one.
9 Sep 2008 4:44 AM #57
- Join Date
- Mar 2007
- Frederick MD, NYC, DC
- Vote Rating
I don't see how allowing for any text input to be a bug. I see it from a point of flexibility.
There are two issues - hackers and users. If a user decides to put arbitrary HTML fragments into an input field, the developer *should* code for it.
Hackers can **easily** thwart/bypass any client side html encode/decode, thus you don't gain security by using encode/decode.
9 Sep 2008 4:47 AM #58
9 Sep 2008 5:06 AM #59
jgarcia, I agree that it's very good to be able to write any html to buttons and such. I only meant to criticize the method/property *names* that are misleading.
However, I think that in 99% of cases a button only has plain text. To make the Button class *easy for most use and still secure* I'd make it encode the html as default, and then add a boolean hasHtml property. The same goes with a Grid cell, Grid Column etc.
Do you have a suggestion on how to fix the '<b>foo' bug "the right way"?
I'm only suggesting that the server should send the data as-is and the client would encode it before rendering, but not when editing it. When saving the data, the client would send it as-is to the server. Thus, everything that has to do with encoding would happen on the client side. This isn't a broken scenario, right? Remember that I'm talking about plain-text data here - something that isn't meant to be rendered as html at all.
9 Sep 2008 5:13 AM #60
Would you care to elaborate on this one?
Suppose that the user needs to enter '<b>foo' in a field and get it back as '<b>foo', verbatim. Are you following me?
I think that's a bit like saying that seat belts increase traffic accidents.
You would feel secure that dont exists and maybe you drive faster due to this fact, but your belt would burst on the smallest burden
Thread Participants: 55
- jack.slocum (4 Posts)
- JeffHowden (2 Posts)
- Animal (1 Post)
- dfenwick (1 Post)
- bidochko (1 Post)
- ApocalypseCow (1 Post)
- firstname.lastname@example.org (9 Posts)
- haibijon (3 Posts)
- dj (1 Post)
- mystix (2 Posts)
- trbs (1 Post)
- Hani (6 Posts)
- Foggy (5 Posts)
- evant (1 Post)
- 72 (1 Post)
- Specks (1 Post)
- ElliotS (1 Post)
- danp (7 Posts)
- hendricd (4 Posts)
- joeri (21 Posts)
- watrboy00 (1 Post)
- jaanus (1 Post)
- kmbarr (2 Posts)
- Lobos (1 Post)
- fallenrayne (3 Posts)
- jburnhams (1 Post)
- dorgan (2 Posts)
- mitchellsimoens (1 Post)
- ofir (2 Posts)
- outrage (1 Post)
- jpnet (1 Post)
- Blackhand (3 Posts)
- naapuri (15 Posts)
- Chris503 (1 Post)
- caustic (8 Posts)
- SimoAmi (1 Post)
- mschwartz (4 Posts)
- edspencer (3 Posts)
- Chods (3 Posts)
- DiscoBoy (3 Posts)
- stephen.friedrich (1 Post)
- Tom23 (1 Post)
- Mike Robinson (1 Post)
- hjones (2 Posts)
- ecassady (1 Post)
- normanrichards (11 Posts)
- olo (1 Post)
- ad_ (3 Posts)
- MrSparks (5 Posts)
- Jan (HL) (1 Post)
- skirtle (2 Posts)
- bfelaco (2 Posts)
- metalinspired (2 Posts)
- similian (4 Posts)
- timmc_basis (1 Post)