jeemok commented 5 years ago

API Explorer for LoopBack 3 is built on top of swagger-ui version 2.x which is no longer maintained. While there are known security vulnerabilities in swagger-ui, we believe they don't affect LoopBack users. See A note on swagger-ui vulnerabilities in README for more details.

We would love to upgrade our (LB3) API Explorer to v3 of swagger-ui, but unfortunately such upgrade requires too much effort and more importantly addition of new features to LB3 runtime, which would break our LTS guarantees.

We are keeping this issue open to keep the discussion in a single place.

Original issue description is below the line.

It may be too much of an effort to upgrade to v3 of swagger-ui as concluded in this issue https://github.com/strongloop/loopback-component-explorer/issues/254

There are two new vulnerabilities reported today, I'm using the following dependencies:

    "loopback": "^3.26.0",
    "loopback-boot": "^2.28.0",
    "loopback-component-explorer": "^6.4.0",
    "loopback-component-storage": "^3.6.1",
    "loopback-connector-mongodb": "^4.2.0",
    "loopback-connector-rest": "^3.4.1",
    "loopback-connector-soap": "^5.0.0",

and it is giving me vulnerabilities warning when I run npm audit:

Moderate Reverse Tabnapping

Package swagger-ui

Patched in >=3.18.0

Dependency of loopback-component-explorer

Path loopback-component-explorer > swagger-ui

More info https://nodesecurity.io/advisories/975

Moderate Cross-Site Scripting

Package swagger-ui

Patched in >=3.20.9

Dependency of loopback-component-explorer

Path loopback-component-explorer > swagger-ui

More info https://nodesecurity.io/advisories/976

Will this be fixed? or do probably ignore them (could use a library to do that) for now?

cs-akash-jarad commented 5 years ago

https://github.com/strongloop/loopback-cli/issues/90

dhmlau commented 5 years ago

@bajtos, since you're looking in this, I'm assigning this issue to you. Thanks.

bajtos commented 5 years ago

As far as I understand these vulnerabilities, they apply only when the attacker can force the swagger-ui instance to load swagger spec created by the attacker. This is not the case with loopback-component-explorer, where we always use the swagger-spec of the application serving swagger-ui. Therefore I believe the vulnerabilities are not applicable to LoopBack.

As for Moderate Reverse Tabnapping, I opened a pull request to back-port the fix to swagger-ui version 2, see https://github.com/swagger-api/swagger-ui/pull/5419

Regarding Moderate Cross-Site Scripting: I did a quick search through swagger-ui 2.x codebase and found this place that may need a fix: https://github.com/swagger-api/swagger-ui/blob/c7439c79137e1e42dc67998f97d710996ca42ce6/src/main/javascript/view/AuthView.js#L125

shockey commented 5 years ago

@bajtos, you're correct -- these vulnerabilities are not relevant if you trust the OpenAPI document that is being provided to Swagger UI, and any webpages that the document links to.

mgleland commented 5 years ago

given that swagger won't take your PR, what can people do to get around this?

jeemok commented 5 years ago

@mgleland there are two ways I can think of for handling this now

Since this vulnerability isn't relevant as discussed, we can filter it off by: npm audit --audit-level high. However, it might also filter other vulnerabilities lower than level high
Or install a package to specifically ignore a vulnerability, eg: node node_modules/better-npm-audit audit -i 975 using better-npm-audit

bajtos commented 5 years ago

Alternatively, you can create your own fork of swagger-ui and tell the explorer to use your copy instead of the built-in one. See the uiDirs option in https://github.com/strongloop/loopback-component-explorer#options.

bajtos commented 5 years ago

https://www.npmjs.com/advisories/985

Versions of swagger-ui prior to 3.0.13 are vulnerable to Cross-Site Scripting (XSS). The package fails to sanitize YAML files imported from URLs or copied-pasted. This may allow attackers to execute arbitrary JavaScript.

LoopBack's API Explorer does not allow clients to import swagger spec from YAML URL/pasted-content. I believe that means loopback-component-explorer IS NOT AFFECTED by this vulnerability.

fuyili commented 5 years ago

any plan to address 975 and 976? swagger-api/swagger-ui#5419 doesn't seem to be merged.

bajtos commented 5 years ago

@fuyili I would love to find a way how to remove npm audit warnings when installing dependencies in LoopBack projects, unfortunately I don't see any easy solution :(

First of all, based on our understanding, 975 and 976 are not affecting users of this component. Ideally, either we or the app developers like you should have a tool to tell npm audit to suppress warnings about security vulnerabilities that are not exploitable in this specific settings. nsp, the original adit tool from Node Security Project, used to support that feature via a configuration file. Unfortunately, npm does not - see the discussion in their forum here: https://npm.community/t/please-provide-option-to-ignore-packages-in-npm-audit/403

If you are using API Explorer in development only, and loopback-component-explorer is listed in devDependencies of your project, then in an ideal world, you would be able to tell npm to limit the security audit to production dependencies only. Unfortunately, that option is not available either, see https://npm.community/t/please-support-production-or-only-production-in-npm-audit/3005

The lack of maturity of npm audit is appalling :(

So what other options do we have?

It would be nice to upgrade our API Explorer to v3 of swagger-ui, unfortunately that involves too much effort and requires addition of new features to LB3 runtime, which would break our LTS guarantees. See https://github.com/strongloop/loopback-component-explorer/pull/209. (Additionally, upgrade to v3 of swagger-ui would require a new semver-major version of loopback-component-explorer too.)

I suppose we could fork swagger-ui and start maintaining our own 2.x version. It would mean investing our effort into work that fixes something that's not broken, which is not a great proposition to me. Additionally, by moving from the official swagger-ui module to our own fork, we will stop receiving further reports about security vulnerabilities that may be discovered in the future.

I understand how bad the situation is for LoopBack users, unfortunately I don't see any reasonable solutions how to address the problem :(

Suggestions are welcome!

ivandov commented 5 years ago

Personally, I don't think the approach should be to fork your own version of swagger-ui, the extra support needed there would not be a good model to follow.

Given how downlevel the swagger-ui dependency is in loopback-component-explorer, I would vote for a dependency update to the latest version of swagger-ui.

bajtos commented 4 years ago

Another vulnerability that does not affect LoopBack users: https://github.com/advisories/GHSA-c427-hjc3-wrfw. Re-posting from https://github.com/strongloop/loopback-component-explorer/pull/269:

GitHub advisory CVE-2019-17495

Link: https://github.com/advisories/GHSA-c427-hjc3-wrfw

A Cascading Style Sheets (CSS) injection vulnerability in Swagger UI before 3.23.11 allows attackers to use the Relative Path Overwrite (RPO) technique to perform CSS-based input field value exfiltration, such as exfiltration of a CSRF token value.

Quoting from the disclosure:

We’ve observed that the ?url= parameter in SwaggerUI allows an attacker to override an otherwise hard-coded schema file. We realize that Swagger UI allows users to embed untrusted Json format from remote servers This means we can inject json content via the GET parameter to victim Swagger UI. etc.

LoopBack 3 API Explorer does not suport ?url= parameter, it always loads the Swagger spec file from the LoopBack server serving the Explorer UI. That means loopback-component-explorer IS NOT AFFECTED by this vulnerability.

ivandov commented 4 years ago

Since these swagger-ui vulnerabilities seem to continue to pop up, and there seems to be limited support for updating v2.x of the swagger-ui library, I think it makes some sense to split off into a loopback-scoped package.

However, the significant downside of doing this would mean that any new vulnerabilities exposed on swagger-ui on 2.x may be missed in the loopback-scoped version. This could potentially result in a valid vulnerability existing in the loopback-scoped package that is just not caught in the audit tools.

LB3 doesn't go EOL until Dec 2020, and I still think the best approach would be to update the swagger-ui dependency to v3.x.

Can someone explain the scope of the work needed to be done here and what integration hooks are needed in the loopback libraries? That way the community may be able to help with the effort here. I expect that LB3 will continue having usage even after Dec 2020 due to difficulties in migration to the LB4 typescript framework.

bajtos commented 4 years ago

Can someone explain the scope of the work needed to be done here and what integration hooks are needed in the loopback libraries?

See https://github.com/strongloop/loopback-component-explorer/pull/209.

As far as I understand the situation, the biggest issue is with authentication - swagger-ui v3 changed the way how authentication works. By default, swagger-ui v3 is using Swagger security schema to build the UI allowing the user to provide the access token. LB3 does not emit such schema, it relies on a custom UI widget contributed by LoopBack API Explorer to deal with the access token.

shockey commented 4 years ago

LB3 [...] relies on a custom UI widget contributed by LoopBack API Explorer to deal with the access token

Swagger UI v3 has a component plugin system that would allow LB3 to inject a custom UI widget, and a requestInterceptor that would allow modification of outgoing requests (to attach authorization information that isn't expressed in the OpenAPI document) 🙂

bajtos commented 4 years ago

Thank you @shockey for chiming in. Are there any examples showing how to inject a custom UI widget and a requestInterceptor? Or any other documentation that would help us to figure out how to implement your suggestions?

ffflabs commented 4 years ago

I can tell you two things:

I don't have the solution
I got it working

The UI

Given VSCode provides a swagger-ui-3 preview of any swager.json compliant file, I thought "what the hell" and tried to do a drop-in replacement. I could not. But working or not, the explorer component exposes, like all components, an interface with the footprint:

module.exports = function (loopbackApp, options) {
    ...
}

Which looks and behaves a lot like boot script, albeit the instantiation happens at a latter stage in the boot sequence. This, in combination with swagger-ui-express makes it easy to just mount the new explorer component. E.g.

module.exports = function (loopbackApp, options) {
    const  router = loopbackApp.loopback.Router(),
             swaggerUi = require("swagger-ui-express");

  loopbackApp.use(
    options.mountPath,
    swaggerUi.serve,
    swaggerUi.setup({...swagger/openapi.json})
  );

   loopbackApp.use(router);
}

But we still need to generate the json definition, because a static json file created through lb export-api-def (or is it lb swagger?) is suboptimal. Okay, then, we can use the same approach as the current explorer, and created the otherwise json contents in memory:

createSwaggerObject = require("loopback-swagger").generateSwaggerSpec;

So putting it all together:

module.exports = async function (loopbackApp, options = { mountPath: "" }) {
  const router = loopbackApp.loopback.Router(),
    swaggerUi = require("swagger-ui-express"),
    createSwaggerObject = require("loopback-swagger").generateSwaggerSpec;

  loopbackApp.use(
    options.mountPath,
    swaggerUi.serve,
    swaggerUi.setup(createSwaggerObject(loopbackApp))
  );

  loopbackApp.use(router);
};

I mounted it as http://localhost:3000/swagger_ui

Peek 2020-07-07 09-16

The Auth

As said by @bajtos there is no text input to enter the access token. However we can still login using the same explorer and or its older brother.

auth

Using the cookie-parser middleware (as shown here) you can make the login endpoint set a cookie. (which is a signed one so not the same as the literal access token).

set_cookie

But I said I don't have the solution

I wanted to publish a POC showing the above, but it doesn't work. (401 error). Apparently it's not a matter of setting the access-control-allow-(origin|credentials) header, there is something else.

In my app (truth be told, the app I developed for a customer) it does work. It does work in my app, but I believe it's just a matter of headers+browser restrictions. See, I'm using https + a let's encrypt certificate to access even my local version of the app (whose domain is legit, but the "local.myap.com" subdomain is set through /etc/hosts to resolve to 127.0.0.1). This is the only reason I can think of for the POC not working whereas my app does.

(My app would also return error 401 if accessed from localhost instead of the https local domain)

Except for this problem (the api sets but does not read the cookie in the localhost attempt) my app works fine. Moreover, it's also possible to convert the swagger.json definition into an openapi one, using, for example api-spec-converter and swagger-ui-3 will accept it too

So...

If anybody can make sense out of this I'm willing to throw a few more hours in to publish a POC and then perhaps a component, but I can't figure out how to make this work in localhost as it does with an https domain

hacksparrow commented 4 years ago

Two more Swagger UI vulns:

https://www.npmjs.com/advisories/985

Versions of swagger-ui prior to 3.0.13 are vulnerable to Cross-Site Scripting (XSS). The package fails to sanitize YAML files imported from URLs or copied-pasted. This may allow attackers to execute arbitrary JavaScript.
https://www.npmjs.com/advisories/976

Versions of swagger-ui prior to 3.20.9 are vulnerable to Cross-Site Scripting (XSS). The package fails to sanitize URLs used in the OAuth auth flow, which may allow attackers to execute arbitrary JavaScript.

These also require crafting custom apispec files, since loopback-component-explorer loads a local file generated by the app, it IS NOT AFFECTED by these vulnerabilities.

ffflabs commented 4 years ago

I left a working example at https://github.com/ffflabs/loopback-swaggerUI4-example

I doubt there would be any interest, but I can encapsulate most of its logic in a package and publish it as a loopback component

strongloop / loopback-component-explorer

swagger-ui vulnerabilities #263

GitHub advisory CVE-2019-17495

The UI

The Auth

But I said I don't have the solution

So...