Regression: XSS in url param

[gabriellupu]

The issue #1262 addressed a problem with XSS in url. In v2.1.0 it was fixed indeed, but it was reintroduced by v2.1.0-M1 and still present in the last version ( v2.1.8-M1 )

https://github.com/swagger-api/swagger-ui/blob/v2.1.0-M1/dist/swagger-ui.js#L348
https://github.com/swagger-api/swagger-ui/blob/v2.1.8-M1/dist/swagger-ui.js#L364

// where data contains the bad url " http://test<script>alert('xss')</script>"
return $('#message-bar').html(data);

So for some reason the update made in 162cd53 (.html() to .text() ) was lost.

Seems like PR #1530 might solve the problem.

[webron]

2.1.0 is newer than any of the -M releases. Yes, the versioning scheme is a bit confusing. All the -Mx were milestone releases prior to the official one which dropped the -Mx notation.

[gabriellupu]

What about 2.1.3 ?
https://raw.githubusercontent.com/swagger-api/swagger-ui/v2.1.3/dist/swagger-ui.js @ 30881

showMessage: function(data){
    if (data === undefined) {
      data = '';
    }
    var $msgbar = $('#message-bar');
    $msgbar.removeClass('message-fail');
    $msgbar.addClass('message-success');
    $msgbar.html(data);
    if(window.SwaggerTranslator) {
      window.SwaggerTranslator.translate($msgbar);
    }
  },

[webron]

2.1.3 is the latest, yes. So if the issue exists there, it's definitely a regression. Thanks for reporting!

[gabriellupu]

Thank for the feedback @webron . It's present in /dist/swagger-ui.js:30881

$msgbar.html(data);

[gmanfunky]

I think the root of this issue may be the Handlebar templates. We have a few fields where we are likely incorrectly disabling Handlebar's HTML escaping.

<div class="info_description markdown">{{{info.description}}}</div>

should just use double-stash operator like:

<div class="info_description markdown">{{info.description}}</div>

https://github.com/swagger-api/swagger-ui/blob/master/src/main/template/main.handlebars#L4

Could this problem be a hold-over from before swagger-ui supported markdown? I don't know the project very well, but I suspect we might not ever need to avoid Handlebar's auto-escaping.

Reference: http://handlebarsjs.com/#html-escaping

[gabriellupu]

In this case templates mechanism has nothing to do with the problem, since we're talking about the incorrect usage of jQuery's html() function called with user data as argument (described above), causing non-persistent XSS.

$msgbar.html(data)

Perhaps the template not-escaped values can be addressed as a different security issue.

[gmanfunky]

Sorry, my mistake @lupugabriel1 . . I see it now, i confused a couple XSS issues.

[gmanfunky]

Here is an example for this issue.
Note: We can't provide an example using petstore.io because it is currently running v2.1.0.

Serve up the current distribution.

cd dist
python -m SimpleHTTPServer 8000

Open this url in chrome:

http://localhost:8000/?url=<script>alert(atob("SGVyZSBpcyB0aGUgWFNT"))</script>

[gfx]

swagger-ui still has XSS?

Here is an XSS demo: http://petstore.swagger.io/?url=http://api.ma.la/tmp/cors/swi/

[domaindrivendev]

Same issue being reported by users of Swashbuckle - domaindrivendev/Swashbuckle.WebApi#838.

I upgraded it's embedded version to swagger-ui v2.2.0 but the malicious url sample above still trigger XSS. This seems like a critical issue. Is there a plan to address it?

[webron]

Yup. We're in the process of handling a few XSS issues, we'll try to tackle this one in the bunch as well. Thanks for bumping it up.

[fehguy]

See a906cff