1. #131
    Sencha User dorgan's Avatar
    Join Date
    Dec 2007
    Location
    Cocoa, FL
    Posts
    286
    Vote Rating
    -1
    dorgan is an unknown quantity at this point

      0  

    Default


    I am stating that HTML should not be store in the database......php has a nice function called strip_tags() and I am sure other language have the same thing and if not can be implemented....if you want people to be able to enter HTML then give them that ability, but still filter it with strip_tags(), which gives you to give an array of tags to be stripped such as <script>...but if you plan on having multiple front ends, one or more of which do not support HTML then strip out all HTML and give the user the ability to insert tokens such as bbcode and let either the server handling replacing those when it gets the information out again or let the front end do the replacing.


    This "someone" could be anyone...firebug allows you to execute arrbitrary javascript again the current page you are viewing so essentially they could include other javascript files that extend the default ExtJS widgets and get rid of your filtering. It wont do anything to other users, but if you are not doing anything server side to protect your database from malicous code its now in your database.

  2. #132
    Ext JS Premium Member
    Join Date
    Mar 2010
    Location
    Austin, TX
    Posts
    11
    Vote Rating
    1
    normanrichards is on a distinguished road

      0  

    Default


    Quote Originally Posted by dorgan View Post
    I am stating that HTML should not be store in the database......
    This is not necessarily true, and it's completely irrelevant. First, HTML text *is* a valid data type, and there is no reason you shouldn't store it in the database if your type is HTML. When it comes user-generated content (such as forum posts) then some sort of non-HTML markup is a technically superior solution, as you have argued. As true as you and I both believe this to be, it is not really very relevant to the discussion here.

    So, why is it irrelevant? The first reason, as I have stated, is that non-HTML content is not necessarily HTML-safe. It is, by definition, not HTML. Before being added in an HTML document, it must be escaped if your goal is to produce valid HTML. Stripping HTML tags is a blunt and frankly lazy approach to avoiding thinking about the real issue.

    If I set my status to "I hate <script> tags", why should that necessarily be wrong? It *might* be wrong, but it's not necessarily wrong. What is wrong is that when that *should* be a valid piece of text, but a lazy developer strips it out because he doesn't understand where data is coming from and going in his system. He doesn't understand data boundaries or have a respect for the correctness of what he produces, so he lazily bans something and feels happy about his app because it looks like it works. (I highly recommend the "programming by coincidence" section of The Pragmatic Programmer for a more eloquent look at the difference between being correct and just looking correct)


    This "someone" could be anyone...firebug allows you to execute arrbitrary javascript again the current page you are viewing so essentially they could include other javascript files that extend the default ExtJS widgets and get rid of your filtering. It wont do anything to other users, but if you are not doing anything server side to protect your database from malicous code its now in your database.
    Executing arbitrary javascript in YOUR browser had no impact on anyone else. You even say this. If it has no impact on your system then, well, it has no impact on your system. The issues with your database are not related. The raw data, the correct and perfectly valid data such as "I hate <script> tags" should be able to be stored in your database without issue. Will javascript in your database be executed by your database? I sure hope not. Proper encoding in the presentation tier is completely orthogonal to the sql injection straw man.

    I would hope that you are using prepared statements and not using any db functions where sql injection is even a theoretical possibility. If it is, please consider moving to a sane framework. SQL injection simply should not even be possible in a reasonably designed app. Why would you consider letting the architecture or functionality of your application be impacted by a an insufficient database access layer. Fix your database layer, and then go back and fix your web application to do the right thing.

    Always be aware of data boundaries and always take the appropriate actions to make sure your data crosses those boundaries CORRECTLY. If you do it correctly, your app will work and all those scary, vaguely understood security issues go away. It won't make your app secure, but it lets you get beyond the junior level every-developer-should-thoroughly-understand-how-to-deal-with-this stuff and actually get to REAL security issues. Seriously people, this is not rocket science. It's one of the most basic concepts of web application development.

  3. #133
    Ext JS Premium Member
    Join Date
    Aug 2007
    Location
    Antwerp, Belgium
    Posts
    555
    Vote Rating
    27
    joeri has a spectacular aura about joeri has a spectacular aura about joeri has a spectacular aura about

      0  

    Default


    Quote Originally Posted by normanrichards View Post
    I would hope that you are using prepared statements and not using any db functions where sql injection is even a theoretical possibility.
    A side note not very relevant to this discussion, but on some databases SQL injection is still possible even with prepared statements and bound variables. An example on oracle would be a stored procedure that constructs a statement based on its argument that is executed via EXECUTE IMMEDIATE. Even if the argument to the procedure is a bound variable, it is still exploitable.

  4. #134
    Ext JS Premium Member
    Join Date
    Aug 2007
    Location
    Antwerp, Belgium
    Posts
    555
    Vote Rating
    27
    joeri has a spectacular aura about joeri has a spectacular aura about joeri has a spectacular aura about

      0  

    Default


    Quote Originally Posted by mschwartz View Post
    Then your other non-web clients should be asking for data responses from your www services in a different format than the web clients. Or those clients can munge the responses to whatever form you need.

    I'm not seeing the issue here.

    No matter how you spin it, the client code is insecure (can be hacked by a malicious user). The server is where you have the ability to control the content.
    Put very simply: all we're talking about here is avoiding XSS issues even with a server that doesn't pre-encode the results. I guess it's a matter of personal opinion about whether you think it's the server's job to encode for the client or not. I think encoding for output is a display layer responsibility, and in Ext the display layer is in the client, so I think the encoding belongs in the client also.

    Besides, encoding on the server doesn't solve my problem. My problem is that I have to explicitly write a lot of encoding statements. If I encode on the server, I have to write exactly as many encoding statements, because there's no way to automate this behavior server-side (unlike Ext it doesn't know which data is meant for output and which for calculation, and so it can't guess how it should be encoded). DRY implies that you should be able to somehow automate this sort of stuff, and Ext seems to me the only place where it can be automated cleanly, because only Ext knows which data needs to be encoded by default.

    Wouldn't it be nice to just let Ext take care of all that encoding? Please take a step back and think about whether the current approach is the most efficient one, because I don't think it is. We can all be more lazy and write fewer lines of code if the change advocated here is implemented.

  5. #135
    Sencha - Ext JS Dev Team Animal's Avatar
    Join Date
    Mar 2007
    Location
    Notts/Redwood City
    Posts
    30,496
    Vote Rating
    44
    Animal has a spectacular aura about Animal has a spectacular aura about Animal has a spectacular aura about

      2  

    Default


    Quote Originally Posted by joeri View Post
    I guess it's a matter of personal opinion about whether you think it's the server's job to encode for the client or not. I think encoding for output is a display layer responsibility, and in Ext the display layer is in the client, so I think the encoding belongs in the client also.
    In the case of an Ext-based RIA, I think that's correct.

    We have extended the domain in which data is "real data" out into the client.

    No longer is it strings of display-data as soon as it's on the wire. We send (encoded in JSON most likely) real data into the client which should behave like the data that it is.

    If the time comes to display that in a user interface, then that is the time to perform encoding.

  6. #136
    Ext JS Premium Member
    Join Date
    Mar 2010
    Location
    Austin, TX
    Posts
    11
    Vote Rating
    1
    normanrichards is on a distinguished road

      0  

    Default


    Quote Originally Posted by Animal View Post
    If the time comes to display that in a user interface, then that is the time to perform encoding.
    Bingo!

    So far I haven't found anything in extjs that hasn't been relatively easy to make encode things correctly. The real challenge has just been figuring out what to do. There's a lot of noise on the forums, and I've really had to dig through the docs to figure out what the options are. It's definitely an area for user education. After I finish going over the apps I'm working on, I'm definitely going to blog about what I had to do, at least for the subset of extjs I'm using.

  7. #137
    Sencha - Community Support Team edspencer's Avatar
    Join Date
    Jan 2009
    Location
    Palo Alto, California
    Posts
    1,939
    Vote Rating
    7
    edspencer is a jewel in the rough edspencer is a jewel in the rough edspencer is a jewel in the rough

      0  

    Default


    Quote Originally Posted by normanrichards View Post
    Bingo!

    So far I haven't found anything in extjs that hasn't been relatively easy to make encode things correctly. The real challenge has just been figuring out what to do. There's a lot of noise on the forums, and I've really had to dig through the docs to figure out what the options are. It's definitely an area for user education. After I finish going over the apps I'm working on, I'm definitely going to blog about what I had to do, at least for the subset of extjs I'm using.
    Sounds good, I look forward to seeing your findings.
    Ext JS Senior Software Architect
    Personal Blog: http://edspencer.net
    Twitter: http://twitter.com/edspencer
    Github: http://github.com/edspencer

  8. #138
    Ext JS Premium Member
    Join Date
    Oct 2009
    Location
    Melrose, MA
    Posts
    48
    Vote Rating
    3
    hjones is on a distinguished road

      0  

    Default


    Has any progress been made on this front?

    I am considering solving the problem in grids by doing the following at the Field level using the convert functionality.

    Code:
    	Ext.data.Types.AUTO = 
    	Ext.data.Types.STRING = {
            convert: function(v) {
                if (v == undefined || v === null)
                    return '';
                return Ext.util.Format.htmlEncode(v);
            },
            sortType: Ext.data.SortTypes.asUCString,
            type: 'string'
    	};
    This operates on the Types used in the fields and the convert method is applied as records are constructed from the JSON/XML normal load mechanism.
    I also like that I can change the default AUTO type to behave as a string which for our app is what the developer assumes it is - this fixes grid sorting and grouping behavior when the type on a field is not specified (auto).

    This approach assumes that tags and scripts in the data coming from the server is NOT valid...our particular use case.
    In addition to this its quite nice to add types for URL and PARTIAL_URL along with any other formats you need.

    This to me is a cleaner way of doing it instead of having to change the column renderer all over the place.

    Thoughts?

  9. #139
    Ext JS Premium Member
    Join Date
    Oct 2009
    Location
    Melrose, MA
    Posts
    48
    Vote Rating
    3
    hjones is on a distinguished road

      0  

    Default


    Quote Originally Posted by anger0805 View Post
    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.
    You can't stop this kind of non-persistent XSS - at least thats my understanding and has been explained many times in this thread.
    I'm talking about persistent XSS - a user being able to get that tag or script in to the system that then gets sent and executed on the client browsers of all the other users in the system.

    So if your form does not stop it AND the server doesn't either then it will get stored in the db and served back up to ALL the other users, i.e. scripts execute and tags get processed on the client browsers.

    Adjusting the Types as I suggest stops any scripts or tags that HAVE entered the system from affecting other user's grids. Any unwanted script/tags are encoded/stripped BEFORE it even gets in the store...and the renderer approach (mentioned in previous posts) can be left to providing custom renderering rather than handle this security issue.

    Now this is a stop gap...for my project it makes sense to validate at the server side to stop tags and scripts. If you expose a RESTful API (an API that clients could access directly) validation on the server is a must.

    In this scenario its hard to understand why the server side would not be expected to completely validate and protect access to the system. From my point of view I find it hard to believe that the developers would not know exactly what is and isn't allowed as input to a system - if this is the situation then you might have bigger problems where security should be lower down the list of things to do.

    The Ext types mechanism provides an extensible way to constrain and validate your data...and since it is always active (readers always use the convert function) the additional overhead is minimal.

    I'd love to see real use cases where tags AND scripts are being passed back to the server and persisted. Its one thing allowing markup to be sent across but scripts too? I'd bet those usecases are in the minority of all the ExtJS-based applications out there.
    From reading this entire thread and related ones in this forum I'm yet to see it.

    Given that Types.AUTO causes problems with sorting (none) and grouping I'd be more in favor of changing AUTO to be the same as String and then creating an explicit HTML type (and perhaps a HTML_WITH_SCRIPT) that the developer HAS to set to allow the 'more dangerous' content through to the grid store. Obviously this wouldn't be backwards compatible (changing AUTO)...an alternative would be to provide a default type to the Record when a type on the field isn't specified.
    Last edited by hjones; 23 Jul 2011 at 6:29 AM. Reason: persistent / non-persistent

  10. #140
    Ext GWT Premium Member
    Join Date
    Jul 2011
    Posts
    2
    Vote Rating
    0
    bfelaco is on a distinguished road

      0  

    Question Has anything been done about this in Ext 4?

    Has anything been done about this in Ext 4?


    I am 100% of the opinion that the grid control should not by default insert data from the model as HTML content in the grid, unless a developer explicitly configures Ext to interpret the content as an HTML fragment. By default, all data coming from anywhere (client or server) should be considered plain text, and all plain text should be handled with care. This is not just an XSS issue, it is a functional issue and a matter of good framework design. The fact is today, the grid is inconsistent with other classes that can be bound with the data-store which makes this much more difficult than necessary. Nor is this a performance issue - the code simply should be storing the content in the DOM by creating a text element rather than using the "innerHTML" attribute. On the contrary, I think what Ext is doing by default today is actually performing worse than if the strings were inserted as text.

    I see no hint that in the API docs that this has been addressed in Ext 4 in any way, or am I just missing something? If not, then when do you plan to address this?