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

XSS vulnerability in https://select2.github.io/examples.html #4587

Closed
Je1te opened this issue Sep 15, 2016 · 14 comments
Closed

XSS vulnerability in https://select2.github.io/examples.html #4587

Je1te opened this issue Sep 15, 2016 · 14 comments

Comments

@Je1te
Copy link

Je1te commented Sep 15, 2016

Under "Loading remote data", the data that is retrieved remotely is not sanitized before displaying it, and this should be done to prevent XSS issues. For example, when I enter je1 in the input field, one of the retrieved GitHub repo's data contains malicious JS code that is executed and that simulates a fake PayPal page which asks for your credit card info (seems like phishing).

@kevin-brown kevin-brown added this to the 4.1.0 milestone Sep 16, 2016
@inty
Copy link

inty commented Oct 7, 2016

We're having this issue with 3.4.8 but it seems to be present in newer versions too. I even get this on the example page...

select2

@kevin-brown
Copy link
Member

I should note that when this was reported, I did get in contact with GitHub about that repository. The repository has since been disabled: https://github.com/accountupdates/paypal

The problem here which we still need to fix is that the example formatter does not properly use jQuery to sanitize the data, and it's something we're planning on fixing in the next release.

@TheThemeBuilders
Copy link

@ Kevin

Viewing the release log at https://github.com/select2/select2 it appears you have not yet attended to this. The 4.1 milestone shows only 14% complete and this matter still open.

As it appears the 4.1 milestone is at least a year away, is it perhaps warranted to have a commit with this vulnerability plus whatever else you have closed?

@kevin-brown
Copy link
Member

@TheThemeBuilders I'm in the process of looking for new maintainers for Select2, and that's been a slow process. If you're interested in joining the (currently small) team of maintainers, get in contact with me and we can chat.

Basically, until that happens I can't guarantee when the next release will be.

@stale
Copy link

stale bot commented Mar 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status: stale label Mar 13, 2019
@snipe
Copy link

snipe commented Mar 19, 2019

Hi all - running 4.0.3 here, and still seeing this issue. Can anyone tell me if there's a fix available in future releases?

And thanks again for a great library!

@stale
Copy link

stale bot commented May 18, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status: stale label May 18, 2019
@pioto
Copy link

pioto commented May 18, 2019 via email

@stale stale bot removed the status: stale label May 18, 2019
@kevin-brown
Copy link
Member

Closing this off as a duplicate of #5448.

I thought I did that earlier...

@kevin-brown kevin-brown removed this from the 4.1.0 milestone May 20, 2019
@bardware
Copy link

bardware commented Aug 1, 2019

I'm using select2 version 3.5.4 in my project and a security report mentioned this very issue as possible risk for an XSS attack.
This issue is marked as a duplicate of #5448. The commits mentioned there fixing the issue solely refer to the documentation.
Early in this issue someone stated they are having this issue with 3.4.8.

Simply put: is there a risk in the library as such that needs a correction in file select2.js?

@kevin-brown
Copy link
Member

Simply put: is there a risk in the library as such that needs a correction in file select2.js?

No.


There was an XSS vulnerability that existed because of a misconfiguation within the Select2 documentation website. Thanks to the fact that people blindly copy/paste from the source code on the documentation website (as well as the examples, some of which had an issue as well), this was referenced in a CVE where someone has misconfigured Select2 to enable XSS within their application.

I am willing to say this though: if you are turning escapeMarkup into a no-op, then you shouldn't be surprised when you are allowing XSS through. The escapeMarkup option exists to automatically escape the text and content within options to mitigate any XSS issues.

@snipe
Copy link

snipe commented Aug 7, 2019

I am willing to say this though: if you are turning escapeMarkup into a no-op, then you shouldn't be surprised when you are allowing XSS through

If you're using custom formatters with rich menus, there isn't really another way to handle this though. It loads up originally escaped properly using the escaping code we added, and then if you select something else and re-select, it was overriding our mitigation. So I'd still say this is a bug or at least a misfeature.

We've resolved this on our end, but it's still broken in the library according to my definition.

@kevin-brown
Copy link
Member

If you're using custom formatters with rich menus, there isn't really another way to handle this though.

I fully disagree. Since 4.0.0, we have supported returning actual DOM or jQuery elements which contain the escaped markup, and that's what we switched the documentation to recommend. I acknowledge that having the documentation recommend an insecure way was a poor choice and one which was easily corrected, but that doesn't mean that there was no way to do it securely.

We've resolved this on our end,

I would highly recommend switching to returning DOM elements from your templates, which will allow you to safely inject text (and only text) into the DOM elements, instead of returning HTML as text directly from your templates.

If you're looking for an example, you can reference the new docs: https://select2.org/selections#templating

@phillbaker
Copy link

phillbaker commented Nov 20, 2020

For anyone coming to this maintaining old/legacy code, the documentation in https://select2.github.io/select2/ might be a bit off in that it has the declaration of formatResult and formatSelection as formatResult(object, container, query) and formatSelection(object, container) but the arguments list contains a fourth/third argument of escapeMarkup. That function is a library supported way to escape markup in the result.

Using that function seems like a more comprehensive approach than the commit linked to above. For example:

formatResult(object, container, query, escapeMarkup) {
  return escapeMarkup(object.foo);
}

civicrm-builder pushed a commit to civicrm/civicrm-core that referenced this issue Sep 7, 2023
Strings coming from system metadata such as "icon" and "color" are generally assumed to be safe,
but out of an abundance of caution this duly escapes them.
Also see select2/select2#4587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants