Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkboxradio's label text being evaluated as HTML on refresh #2101

Closed
Elkano opened this issue Jul 8, 2022 · 3 comments · Fixed by #2102
Closed

Checkboxradio's label text being evaluated as HTML on refresh #2101

Elkano opened this issue Jul 8, 2022 · 3 comments · Fixed by #2102
Assignees
Milestone

Comments

@Elkano
Copy link

Elkano commented Jul 8, 2022

If you generate a Checkboxradio from a checkbox/radio with a label that contains encoded HTML, e.g. <em>test</em> this will work fine at first.
If however a refresh is triggered on that instance (explicitly or e.g. by turning it into a Controlgroup), the previously escaped HTML will now be evaluated.

If the label was created based on some user input, this could lead to unexpected code execution even though the initial output was escaped.

Example:
https://jsfiddle.net/69krwj75/

This is caused by the initial label being read as text (and thus decoded) for text nodes.

that.originalLabel += this.nodeType === 3 ? $( this ).text() : this.outerHTML;

When the label is later updated, it is however done via append.

this.label.append( this.options.label );

A fix would likely be to read the initial label text as HTML as well to preserve the encoding of the entities, however the likely was some reason to do things the way they currently are.

Judging from the blame, this bug has existed since the initial code from 8 years ago.


I'm aware that the teams resources are limited but I wanted to at least report the bug.
As a workaround, one can wrap their text into a <span> and thus causing it to be read via this.outerHTML instead.

@mgol mgol added this to the 1.13.2 milestone Jul 14, 2022
@mgol mgol self-assigned this Jul 14, 2022
mgol added a commit to mgol/jquery-ui that referenced this issue Jul 14, 2022
If you generate a Checkboxradio from a checkbox/radio with a label that
contains encoded HTML, e.g. `&lt;em&gt;test&lt;/em&gt;` this will work fine
at first. If, however a refresh is triggered on that instance (explicitly or
e.g. by turning it into a `Controlgroup`), the previously escaped HTML will
now be evaluated.

If the label was created based on some user input, this could lead to
unexpected code execution even though the initial output was escaped.

Fixes jquerygh-2101
mgol added a commit that referenced this issue Jul 14, 2022
If you generate a Checkboxradio from a checkbox/radio with a label that
contains encoded HTML, e.g. `&lt;em&gt;test&lt;/em&gt;` this will work fine
at first. If, however a refresh is triggered on that instance (explicitly or
e.g. by turning it into a `Controlgroup`), the previously escaped HTML will
now be evaluated.

If the label was created based on some user input, this could lead to
unexpected code execution even though the initial output was escaped.

Fixes gh-2101
Closes gh-2102
@mgol
Copy link
Member

mgol commented Jul 14, 2022

Thanks for the report! PR: #2102. We've just released it as part of 1.13.2: https://blog.jqueryui.com/2022/07/jquery-ui-1-13-2-released/

I'll request a CVE shortly.

In the future, please direct security reports at [email protected] instead of GitHub issues so that they are not immediately visible to the whole world. I'll add the SECURITY.md file to this repo to make this policy clearer; we already have one for jQuery but we missed one for UI.

@mgol
Copy link
Member

mgol commented Jul 17, 2022

@Elkano I submitted a security advisory at:
GHSA-h6gj-6jjq-h8g9
and added you in the Credits section; if you want to show up there, you’ll need to approve.

@mgol
Copy link
Member

mgol commented Jul 18, 2022

The issue now has its CVE identifier: CVE-2022-31160.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants