bootstrap-vue: Script tag is not escaped when using :option="" in form-select

I was trying to do some XSS in my project and I got some un-shown text. Which I found out that it was the cause of the script tag that was not escaped… So I tried doing a v-for loop inside an <option> tag and use {{ }} (Double brackets) to escaped the script tag. I would like to make a pull request sadly bootstrap_vue is using a render less components (I don’t really know but it’s using render function) which I don’t have much knowledge about. So I just submitted an Issue here.

by the way you can replicate the problem using the Live documentation on Bootstrap_vue site.

My browser: Opera Operating System: Fedora LXDE

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 2
  • Comments: 17 (4 by maintainers)

Commits related to this issue

Most upvoted comments

@pi0 i have already changed my code to use slots after discovering the vulnerabilities in my app. so actually it is up to you and the team to decide on whether to minimize changes in bootstrap-vue or the likelyhood of other developers to introduce vulnerabilities in her/his app (i would opt for the latter if i were to decide).

i agree with your point in the clarification letter that separating trusted from untrusted input is not (and cannot be) the task of bootstrap-vue (and hence any html sanitization - including removal of <script> tags, which is neither necessary nor sufficient - can be dropped), but providing separate channels for trusted (html) and untrusted (text) data is important, and it vould avoid confusion and wrong expectations if the respective properties were named and documented accordingly. but clearly, the two channels are already there: the text property for trusted html and the slot for untrusted text.

imho: nope! app developers would then end up doubling their code for sanitizing all texts…

Seriously, see e.g. the description of v-html in the vuejs doc: https://vuejs.org/v2/guide/syntax.html#Raw-HTML

If you have a property called options.text you are alluding this is interpreted as text and hence passed to a text-like property (domProps.textContent in this case). Otherwise it should be called options.html (which could exist simultaneously and be passed into innerHTML. Otherwise, this should be clearly documented to avoid security risks (I assume more than 90% of app developers who pass in user input will not sanitize it and have a security vulnerability in their app). Please feel free to consult resources of the security folklore to make an informed decision - but at least add some documentation that the property options.text is not secure (i.e. treated as HTML - similar to the remarks for v-html) and explicit options should be used instead if the content is untrusted (e.g. select controls filled from database content and/or user input).

this occurs in b-form-radio-group as well as in b-form-select. please note that this is a severe security problem!

@CAR7OGRAPH3R see conversation above - this was exactly my suggestion - unfortunately to no avail. the work-around is to use <b-form-select>{{ my content goes here }}</b-form-select>.

note also that other controls may be affected, too, which has been established for b-form-radio-group, but there may be others, which have not been investigated yet. therefore it is imho strongly advisable to perform a thourough vulnerability testing whern uising this lib.

imho sanitizing html is a difficult task to say the least. there are many ways to circumvent sanitizing, some of which are easy to be found, like:

https://support.portswigger.net/customer/portal/articles/2590821-xss-beating-html-sanitizing-filters https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet

but there are numerous others. it is not only necessary to handle each in the sanitizing code, but also (recurrently!) in testing, which is a big effort and also error prone. therefore i prefer the easy and deterministic method of not treating untrusted content as html. period.

there are sources on handling untrusted content (with some hints on esacping html), like:

https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet

Hi @tmorehouse

Apologies for commenting on an issue that has already been closed - please let me know if you would prefer a new issue be raised instead.

An XSS vulnerability was recently found in one of our applications that uses a b-form-select element populated from user supplied data ( as mentioned above ).

I have simplified and replicated the scenario ( that does not use <script> tags ) here.

When speaking with the developers ( who admit the validation of this data can and will be improved ), their understanding was that the data would only ever be treated as text, not markup.

OWASP recommend never providing untrusted data to innerHTML, and to use textContent instead - would this be possible?

@tmorehouse I have had a look at the source code of form-select. I think, the problem is in line 24 of b-form-select.js, which is currently:

domProps: { innerHTML: option.text, value: option.value }

this means, the (untrusted) data contained in option.text is directly used as innerHTML, which allows to inject JavaScript code using the payload I have supplied above. Assuming the text is fed from a database that contains user entries, a malicious user is able to insert code that is executed in another user’s browser. The line should imho be something like:

domProps: { textContent: option.text, value: option.value }

see also https://github.com/vuejs/vue/blob/master/src/platforms/web/server/modules/dom-props.js.

The same applies to form-radio, see line 35 of form-radio-group.js:

[h('span', { domProps: { innerHTML: option.text } })]

should imho be:

[h('span', { domProps: { textContent: option.text } })]

I havent checked whether other components are affected, too, but any use of innerHTML is suspicious - only trusted content should go in there at any rate.