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

Fix code execution vulnerabilities. #2254

Merged
merged 2 commits into from Mar 3, 2015

Conversation

adunkman
Copy link
Contributor

@adunkman adunkman commented Mar 3, 2015

I noticed a scripting vulnerability just now in 1.3.0 when rendering results — and upon checking master, not only is that one present, but there’s also another when rendering a single select’s selected option.

Demo of the issue using chosen@501568a (current master): http://jsfiddle.net/hpv4La79/3/.

Fixes #2132, replaces #2209, #1806. Effectively undoes #1638.

When rendering the selected item for a single-select chosen, be sure to pass an escaped version of the select text (the html property, perhaps confusingly).
When the results container is rendered, be sure to use the escaped version of the contents instead of the unescaped version.
@pfiller
Copy link
Contributor

pfiller commented Mar 3, 2015

Yup - #1638 should not have merged after all. The Ember folks are going to be displeased by this, but it's the right thing to do.

Thanks, @adunkman!

pfiller added a commit that referenced this pull request Mar 3, 2015
@pfiller pfiller merged commit 717d3af into harvesthq:master Mar 3, 2015
@adunkman adunkman deleted the fix-xss-vulnerabilities branch March 3, 2015 21:52
satchmorun pushed a commit that referenced this pull request Jun 22, 2017
Before this change, if you tried to search for "&" to match "Stuff
& Nonsense", what we would do is html-escape _both_ things and compare
the escaped strings.

This works fine in the case where you just type "&". However, if you
type "mp", that will match the html entity "&" in the middle of the
html-escaped text we're comparing it to. Which is incorrect.

Additionally, we highlight our "matched" result. When we do, we break up
the entity so that the resulting HTML looks like:

    Stuff &a<em>mp</em>; Nonsense

And you get to see what basically looks like garbage on the screen.

![screen garbage](https://camo.githubusercontent.com/b5e384f3f622f3560739c287c481e05a7b0668af/68747470733a2f2f646c2e64726f70626f7875736572636f6e74656e742e636f6d2f7370612f7470786279757478626b747069347a2f6c7566336c7835792e706e67)

This commit fixes all of that by just comparing the actual text of
things. We avoid the problem previous addressed in #2254 by escaping
each piece of the to-be-highlighted option text before assembling it
into HTML.
satchmorun pushed a commit that referenced this pull request Aug 22, 2017
Before this change, if you tried to search for "&" to match "Stuff
& Nonsense", what we would do is html-escape _both_ things and compare
the escaped strings.

This works fine in the case where you just type "&". However, if you
type "mp", that will match the html entity "&amp;" in the middle of the
html-escaped text we're comparing it to. Which is incorrect.

Additionally, we highlight our "matched" result. When we do, we break up
the entity so that the resulting HTML looks like:

    Stuff &a<em>mp</em>; Nonsense

And you get to see what basically looks like garbage on the screen.

![screen garbage](https://camo.githubusercontent.com/b5e384f3f622f3560739c287c481e05a7b0668af/68747470733a2f2f646c2e64726f70626f7875736572636f6e74656e742e636f6d2f7370612f7470786279757478626b747069347a2f6c7566336c7835792e706e67)

This commit fixes all of that by just comparing the actual text of
things. We avoid the problem previous addressed in #2254 by escaping
each piece of the to-be-highlighted option text before assembling it
into HTML.
satchmorun pushed a commit that referenced this pull request Aug 22, 2017
Before this change, if you tried to search for "&" to match "Stuff
& Nonsense", what we would do is html-escape _both_ things and compare
the escaped strings.

This works fine in the case where you just type "&". However, if you
type "mp", that will match the html entity "&amp;" in the middle of the
html-escaped text we're comparing it to. Which is incorrect.

Additionally, we highlight our "matched" result. When we do, we break up
the entity so that the resulting HTML looks like:

    Stuff &a<em>mp</em>; Nonsense

And you get to see what basically looks like garbage on the screen.

![screen garbage](https://camo.githubusercontent.com/b5e384f3f622f3560739c287c481e05a7b0668af/68747470733a2f2f646c2e64726f70626f7875736572636f6e74656e742e636f6d2f7370612f7470786279757478626b747069347a2f6c7566336c7835792e706e67)

This commit fixes all of that by just comparing the actual text of
things. We avoid the problem previous addressed in #2254 by escaping
each piece of the to-be-highlighted option text before assembling it
into HTML.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XSS vulnerability (new in v1.2.0)
2 participants