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 some XSS with improved handlebar template escaping #1633

Closed
wants to merge 1 commit into from

Conversation

gmanfunky
Copy link

I think this will address many cases of cross-site scripting via swagger api-data. I would love feedback from someone more familiar with the project.

The root of many of our XSS issues appears due to Handlebar templates.

We have a few fields where we are likely incorrectly disabling Handlebar's HTML escaping by using "triple-stash" {{{ }}} (http://handlebarsjs.com/#html-escaping)

Please note: this PR doesn't fix signature.handlebars. The signature feature still needs more attention to fix a related XSS vulnerability while keeping functionality.

Related:
#830

@nikhiljindal
Copy link

I am not an expert, but using HTML escaping seems like the right thing to me.
fwiw, I am sure @fehguy is going to ask you to send the change on develop_2.0 branch.
And I guess you need to change dist/ as well?

@gmanfunky
Copy link
Author

I'm new to gulp stuff and I don't trust my build pipeline a ton (windows while everyone one else is osx these days), so I didn't go for making the dist. I kinda expect that'd be better as a CI build job task, but can give it a go if needed.

It looks like the develop_2.0 branch has been retired in favor of merging to master. https://groups.google.com/forum/#!topic/swagger-swaggersocket/dEwm8J5p7Ag/discussion

@gmanfunky gmanfunky mentioned this pull request Oct 2, 2015
@fehguy
Copy link
Contributor

fehguy commented Aug 10, 2016

i believe this has been handled.

@fehguy fehguy closed this Aug 10, 2016
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.

None yet

4 participants