search

ratiw / vuetable-2

data table simplify! -- datatable component for Vue 2.x. See documentation at
https://vuetable.com
MIT License
2.16k stars 399 forks source link

vuetable-2 allows XSS #466

Open gritsenko-konstantin opened 6 years ago

gritsenko-konstantin commented 6 years ago

Data passed to vuetable-2 output not encoded all data are displayed with v-html. This allows XSS on any data field.

4nik4 commented 6 years ago

According to @ratiw:

The use of "v-html" is necessary to support icon rendering feature and a few others. Switching back to "v-text" or basic interpolation is not an ideal solution.

You could fork the repository and do find/replace "v-html" with "v-text". It should not interfere with anything else other than with aforementioned functionality.

However, I haven't tested this yet.

gritsenko-konstantin commented 6 years ago

may be it would be a good idea to add special cell type?

ratiw commented 6 years ago

I'm really not a security expert here, so my knowledge on this issue is quite limited.

If it is embedded inside the component, I don't really know if XSS is still possible.

I think I can come up with a solution to get rid of all v-html inside Vuetable and its related components, but this will be a trade-off for convenience.

In v2.0 of Vuetable, I think I can provide a field component that will allow displaying icon without using v-html, but it will not be universal, meaning that it will only work with a specific element, let's say <i>. This may not work in every CSS framework. So if it does not work, it has to be re-implemented again for that CSS framework, let's say <span>.

I'm open to discussions and suggestions on this.

gritsenko-konstantin commented 6 years ago

I think v2 has good functionality, and it is good. So as suggestion we can add custom SpecialField which will output escaped data only through v-text. this will fix XSS issues with auto output data from database.

what do you think?

ratiw commented 6 years ago

@gritsenko-konstantin I think adding new field component without removing v-html usage inside Vuetable code is not going to help. I also think we do not understand the XSS well enough.

shaprk commented 6 years ago

XSS works only on client data, therefore you don't need to change v-html on all fields and lose icon rendering. I think validating search field is enough; because that's the only client input I think. If you show its input anywhere, use v-text for it

4nik4 commented 6 years ago

@ratiw could you tell me if anything has been done regards to this in the beta version?

If not, i am thinking of forking the repository for myself and do some html sensitization in v-html.

ratiw commented 6 years ago

@AnikaErceg Nope. I haven't done anything regarding this yet.