Security: Cross-site Scripting (XSS) Attack

[serges147]

w2ui grid control is subject of XSS attack. "records" fields values are insert into HTML as is, without proper escaping.

To reproduce try http://w2ui.com/web/demos/#!grid/grid-21 demo, and insert as a value following text: <script>alert(123)</script>

[vitmalina]

If I understand correctly, the main remedy for XSS attacks is to sanitize user input when it is saved on the server. The security issue is not when user can execute code on the local machine (which you can do any time opening console and executing js), but when a js script is saved into db and then executed on another users machine, allowing hacker to still the session.

I will evaluate what can be done to help developer deal with this issues.

[codeart1st]

The check should be server-sided before data from the grid saved to the the db. It makes no sense doing this client-sided. At most, in addition to server-sided check.

[serges147]

You absolutely right in case of HTML generation on server-side. But imagine if you have generic REST service returning JSON. Such service should return original data as is without any assumption where and how data will be used, so it can't do encoding ahead (on server side).

That is actually my case - I have static HTML, it has w2ui grid which consumes the service. The original "reproduce" scenario in this issue report was just to easily show the general problem.

In my case an user is not able to enter any data by himself, but only see what REST service returned. The user should be able to see and search (with proper highlighting!) whatever data returned from the server (even if data contains tags, scripts or whatever). But at the same time the data from server should not be "executable" in any sense - only visible, searchable and (if needed) editable. So, I don't see how this could be achievable without proper data encoding on client side.

My current workaround is specify "render" function for a column, and there I do something like this:

columns: [{ field: 'name', caption: 'Name', size: '35%', sortable: true,
    render: function (record) {
        return w2utils.encodeTags(record.name);
    }

It works (prevents data "execution"), but search highlighting completely goes wrong as soon as you have < or > symbols is your data. In such case entering "lt", ";" text in search breaks data visualization in the grid.

[vitmalina]

It is a common practice for render function (similar as in your example) to return HTML that should be inserted as HTML. It is much more common then inserting it with encoded tags. As such, I think it is proper behavior. If you want to display tags, then doing what you doing is the way to go and as you see, it is just with a few lines of code. As for highlight, you are right, there are some bugs with it and it needs to be fixed.

I do not see here a security issue. I am of the opinion that data needs to be sanitized when it is saved and it should be server side sanitation. I think it is logical to assume that whatever your server returns is trusted data. If you assume that your server data is not trusted, then you have a bigger security concern that all the data (which might include sensitive), on the server is compromised and that need to be addressed.

[serges147]

Again, you absolutely right about sanitizing HTML rendering on server side, and trust to your server, but... In case of GENERIC service how can you do any sanitizing? Imagine a server has "Items" service, and an item has weird name like "<script..." - why not?! As generic service it should return data as is, and NOT as "&lt;script..." or "script..." because it is generic and not intended only for HTML - may be I will consume the service in C# code.

Speaking about trusted server I would like to add that it's quite common to AJAX 3rd party public data feeds instead of your own server, so in such case you can't trust to these data.

OK, generally I understood that it is not a BUG - it is a FEATURE of the grid :) My main point that I currently see in the grid strong coupling between rendering and business logic (like searching data for highlighting). With such coupling I don't surprise that "...there are some bugs..." - the w2marker() function is a "good" example. IMHO better choice if grid will provide facilities for clean separation of final rendering from original data and logic. In such case it's quite possible that rendering will be simpler, business logic without bugs and ready for any arbitrary data, and there will be no (potential) security issues.

BTW, I would like to thank you for the great library! I very like it, and I'm just trying to do constructive critics ;) Thanks!

[vitmalina]

Appreciate your comments, will definitely keep this in mind to come with some solution.

[dv1x3r]

I also consider this as a major issue. It is not only about XSS attacks. This also makes it impossible to edit inline text starting with angle bracket. For example: <item.

This library is great and beautiful, but it blindly renders everything.

So for security reasons make sure you use row => w2utils.encodeTags(row.field) for every text field, and do not allow inline editing for text fields.