Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix/xss issues on data attributes #27047

Merged

Conversation

don-spyker
Copy link

@don-spyker don-spyker commented Aug 10, 2018

fix of xss issues that have been fixed in v4.x already + fix of two additional xss issues

Fixes #26625 / CVE-2018-14040
Fixes #26628 / CVE-2018-14042
Fixes #27044 / CVE-2018-20676
Fixes #27045 / CVE-2018-20677

@don-spyker don-spyker closed this Aug 10, 2018
@don-spyker don-spyker reopened this Aug 10, 2018
@don-spyker don-spyker force-pushed the fix/xss-issues-on-data-attributes branch from 5fd1134 to ae29b6a Compare August 10, 2018 12:41
@don-spyker don-spyker mentioned this pull request Aug 10, 2018
@@ -16,7 +16,9 @@
var Affix = function (element, options) {
this.options = $.extend({}, Affix.DEFAULTS, options)

this.$target = $(this.options.target)
var target = this.options.target === Affix.DEFAULTS.target ? $(this.options.target) : $(document).find(this.options.target)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for your ternary here: $(document).find(this.options.target) is enough

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first, but that broke the functionality as the default is "window" and $(document).find('window') didn't work while $(window) evaluates correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I understand 👍

Copy link
Member

@Johann-S Johann-S left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need unit test for each plugins you changed

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants