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

Fixed double-encoding on Select2 dynamic drop-down menus with XSS protection #9079

Merged

Conversation

uberbrady
Copy link
Collaborator

As noted by the author of select2, the more-secure way of creating
rich Select-dropdowns is to use jquery to create HTML snippets and
carefully modify text attributes within there. This prevents any
XSS from being brought to the page. As a side-effect, the extra
escaping that we had to do in all of the internal selectlist calls
is now no longer necessary, and has been removed. Rebased and
squashed from the original.

I still need to build assets, and I think this implicitly has a regression where it no longer displays alt-text for images - but I'll add commits for that.

As noted by the author of select2, the more-secure way of creating
rich Select-dropdowns is to use jquery to create HTML snippets and
carefully modify text attributes within there. This prevents any
XSS from being brought to the page. As a side-effect, the extra
escaping that we had to do in all of the internal selectlist calls
is now no longer necessary, and has been removed. Rebased and
squashed from the original.
@uberbrady uberbrady marked this pull request as ready for review February 2, 2021 23:49
@uberbrady uberbrady removed the backend label Feb 2, 2021
@snipe snipe merged commit 9a224a0 into snipe:develop Feb 2, 2021
@snipe
Copy link
Owner

snipe commented Feb 2, 2021

This looks great, thank you so much @uberbrady. So sorry your original PR for v5 was such a hassle.

@snipe snipe changed the title Modified how we do Select2 dynamic drop-down menus to be more secure Fixed double-encoding on Select2 dynamic drop-down menus with XSS protection Feb 2, 2021
@ItsANoBrainer
Copy link

This looks like it has broken the "number to display" dropdown in the models section. Report here:

#9085

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

Successfully merging this pull request may close these issues.

None yet

3 participants