Swagger UI Description allows HTML Injection (XSS by Design)

[DinisCruz]

Hi, I was trying out the Swagger UI, and I noticed that the swagger.apiInfo.description value allows HTML, which could lead to an XSS injection (if that description string is maliciously manipulated)

Here is a unit test that replicates this issue (in Coffee-Script):

QA_NWR_API = require './TM-QA-NWR-API'
app        = require '../../app'


describe.only 'swagger', ->
  page = QA_NWR_API.create(before, after)
  url = 'http://localhost:8002/'
  server = null

  before (done)->
    server = app.listen(8002);
    url.append('docs').GET (html)->
        html.assert_Is_String()
        done()

  after ->
    server.close()
    url.GET (html)->
        assert_Is_Null(html)
        done()

  it 'Issue 830 - Swagger UI Description allows HTML Injection (XSS by Design)', (done)->
    dynamicText = 'jQuery Injection - '.add_5_Random_Letters()
    javascript = "$(function() { $('#target').html('#{dynamicText}')})"
    app.swagger.apiInfo.description = "Description element allows: <h1 id='target'>...</h1> <script>#{javascript}</script>"
    page.chrome.open url + 'docs', ()->
      page.html (html,$)->
        $('#target').html().assert_Is(dynamicText)
        done()

... as executed in WebStorm:

It looks like this is 'by design' since for example the swagger-node-express takes advantage of this 'feature' to add a link to its description: https://github.com/swagger-api/swagger-node-express/blob/master/sample-application/app.js#L83-L90

I don't think that this is a high risk security issue, but it would be good to either:

a) add a note to the documentation (referencing this issue) with an alert to be careful with the swagger.apiInfo.description value, or
b) use markdown for the description value (which will allow features like links, without the html scripting capability)

[DinisCruz]

Here is another XSS Injection location

[DinisCruz]

And a couple more on

which are rendered here:

[fehguy]

should be fixed in #862

[DinisCruz]

Hi, is there a test that confirms the XSS fix?

[mala]

@fehguy @mohsen1
this is too bad way to fix xss https://github.com/swagger-api/swagger-ui/pull/862/files#diff-7b22c617cbad75cdd8123d16345c8e80R2699

example: <img src=x onerror=...>
http://petstore.swagger.io/?url=http://api.ma.la/tmp/cors/swi/

I think that restriction of url param or attention about security is needed.

[mala]

ping @fehguy @mohsen1

[webron]

@mala - feel free to submit a PR.

[mala]

I can write a patch, but I don't know all function of swagger-ui, and I'm not using swagger-ui.
Please reopen this issue for now.

[ianlewis]

Please reopen this issue as it's a full on XSS vulnerability with the ability to exploit it just by having users click on URLs.

[webron]

While we do want to solve security issues with the UI, just reopening this issue won't help. The issue was opened on a specific XSS case. Opening an issue on 'you have XX vulnerabilities everywhere' doesn't help us isolate the problems and solving them, even if there are many of them, unless it comes with a clear method of checking and testing the whole project for such vulnerabilities.

If you'd like to help us further, please open specific issues pointing us to the various locations, or a general one with more information on how to map them out. Suggestions on how to fix the issues are also welcome, and PRs are great too.

[gmanfunky]

The specific case this issue described was not fixed with the initially referenced fix. #862 added a 'sanitize' helper, but it was never put in use.

@DinisCruz issue also goes on to describe that this isn't just a <script injection issue.

To fix most XSS described in this issue, I suggest a combination of more liberal use of handlebar template escaping (here: #1633)

API-Data can use Markdown to insert hyperlinks in descriptions if desired.

[nikhiljindal]

As pointed our earlier, the specific XSS case that this issue was filed for doesnt seem to be fixed yet.
To repro: http://petstore.swagger.io/?url=http://api.ma.la/tmp/cors/swi/ (you should not be getting that dialog)

I will recommend opening this issue until it is fixed.

[nikhiljindal]

One way to fix the reported XSS issue (http://petstore.swagger.io/?url=http://api.ma.la/tmp/cors/swi/) is to just remove the URL query param. Is that query param required?

(I can send a PR if removing it is fine)

[webron]

@nikhiljindal - it's required, yes. It's being used by quite a few users.

[DinisCruz]

btw, to exploit this you don't need user interaction since the payload can be delivered on any page the user visits (in an hidden image)

Ok, after looking at it for a little bit, this is actually a much worse vulnerability than I originally thought of, as it stands it looks like it will affect any website that exposes the swagger interface.

For example : http://kubernetes.io/kubernetes/third_party/swagger-ui/?url=http://api.ma.la/tmp/cors/swi/

That javascript code will now execute under the domain affected (in the case above http://kubernetes.io/ )

I'm sure there is a good way to do a Google Dork search for swagger exposed interfaces

[fehguy]

Fixed in 2.1.5

[DinisCruz]

great thanks

is there a test that confirms the fix?

can you point to the commit that fixed it?