-
Notifications
You must be signed in to change notification settings - Fork 9k
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
Swagger editor v3 description key has a XSS vulnerability #3163
Comments
From @attritionorg on June 1, 2017 2:45 Looks like http://seclists.org/bugtraq/2016/May/15 on first glance, which was apparently fixed in the past. |
Thanks for reporting. This is an issue with swagger-ui. Moving there for further treatment. |
@webron Thank you for putting this in the proper place. |
We're sanitizing our Markdown now - this should be fixed. Thanks for raising the issue @amrtgaber! |
@webron, can you confirm if this is the same issue I linked or different? |
@shockey Hey I might be testing this incorrectly but I still see the issue. I just cloned the Swagger Editor master branch and did an |
@amrtgaber, thanks for checking. Swagger-UI 3.0.12 does not have the fix, it was released last Friday 😄 I'll be doing releases of all the projects this evening, which will contain the fix (presumably, this version will be 3.0.13). If you want to confirm the fix before the release, you can use npm to install the latest master branch as a module: just change your swagger-ui dependency to It looks fixed from my end, but I'd really appreciate it if you'd double-check it. |
Sorry for jumping the gun too early on testing it! I made the Here's the relevant line from Let me know if I'm just doing this all wrong or if there's anything I can do to help. |
@amrtgaber huh, I was just about to ping you. Thanks for retesting. We'll see what @shockey has to say ;) @attritionorg I cannot confirm if it's the same issue or not. This is a completely different code base than the referenced vesion. |
@amrtgaber, looks like I may need to update the "running locally" docs, since they appear to have misled you. my apologies!
alternatively, you can run i added a task for myself to rewrite that part of the readme, as it clearly sent you down the wrong path! as for the XSS vulnerability - we deployed petstore.swagger.io this afternoon, and it appears to be fixed. i put your proof of concept into a gist here: https://gist.github.com/shockey/c67fe0a0fdd213af132360bc5b3a0b07, you can grab the raw url and load it into swagger-ui to confirm. make sure to force-refresh. please request a reopen if you still see it happening after all that, and thank you for diligence in making sure this is fixed! |
@shockey No worries I should've scrutinized the build process more, that makes perfect sense! I did Thanks! You did an excellent job and thank you for walking me through the testing. Sorry I kept missing the mark, I wasn't trying to make your job any harder. 😄 Great work and excellent response time! |
Thanks for bringing up the issue, @amrtgaber. We look forward to future contributions. |
From @amrtgaber on June 1, 2017 0:15
Swagger-ui version
3.x
Vulnerability details
The description field is not guarded against arbitrary javascript execution. Any YAML file can trigger the vulnerability, whether it's imported from url or copied and pasted directly in the editor. Here is an example YAML file that does so:
Security risk
This opens a vector for general cross-site scripting attacks that easily expose sensitive information which can be remotely shared with the attacker (among several other types of risks).
Notes
I tested this vulnerability on http://editor.swagger.io/# by copying and pasting the above YAML file and saw the alert pop up. I hope this helps, let me know if I made a mistake. Also, seems Swagger editor version 2 had to contend with a similar issue: #908.
Copied from original issue: swagger-api/swagger-editor#1348
The text was updated successfully, but these errors were encountered: