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
- Revert "feat(security): Strip HTML script tags before inserting content into DOM. Fixes #1974,#1665 (#2129)" This reverts commit 6dde0cba3f89374fea3cb09b8a6b90363bf04243. — committed to bootstrap-vue/bootstrap-vue by tmorehouse 6 years ago
- Revert "feat(security): Strip HTML script tags before inserting content into DOM. Fixes #1974,#1665 (#2129)" (#2135) This reverts commit 6dde0cba3f89374fea3cb09b8a6b90363bf04243. Reverts #2129 ... — committed to bootstrap-vue/bootstrap-vue by tmorehouse 6 years ago
- feat(security): Strip HTML script tags before inserting content into DOM. Fixes #1974,#1665 (#2134) * fixed a typo (#1931) * Create utils/strip-sripts.js Utility for removing script tags from i... — committed to bootstrap-vue/bootstrap-vue by tmorehouse 6 years ago
- feat(security): Strip HTML script tags before inserting content into DOM. Fixes #1974,#1665 (#2134) * fixed a typo (#1931) * Create utils/strip-sripts.js Utility for removing script tags from injec... — committed to bootstrap-vue/bootstrap-vue by tmorehouse 6 years ago
@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-vueor 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: thetextproperty 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.textyou are alluding this is interpreted as text and hence passed to a text-like property (domProps.textContentin this case). Otherwise it should be calledoptions.html(which could exist simultaneously and be passed intoinnerHTML. 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 propertyoptions.textis not secure (i.e. treated as HTML - similar to the remarks forv-html) and explicit options should be used instead if the content is untrusted (e.g.selectcontrols filled from database content and/or user input).this occurs in
b-form-radio-groupas well as inb-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-selectelement 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 usetextContentinstead - 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 ofb-form-select.js, which is currently:domProps: { innerHTML: option.text, value: option.value }this means, the (untrusted) data contained in
option.textis 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 ofform-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
innerHTMLis suspicious - only trusted content should go in there at any rate.