-
Notifications
You must be signed in to change notification settings - Fork 100
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-ui vulnerabilities #263
Comments
@bajtos, since you're looking in this, I'm assigning this issue to you. Thanks. |
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 swagger-api/swagger-ui#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 |
@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. |
given that swagger won't take your PR, what can people do to get around this? |
@mgleland there are two ways I can think of for handling this now
|
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 |
https://www.npmjs.com/advisories/985
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. |
any plan to address 975 and 976? swagger-api/swagger-ui#5419 doesn't seem to be merged. |
@fuyili I would love to find a way how to remove 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 If you are using API Explorer in development only, and The lack of maturity of 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 #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 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! |
Personally, I don't think the approach should be to fork your own version of Given how downlevel the |
Another vulnerability that does not affect LoopBack users: GHSA-c427-hjc3-wrfw. Re-posting from #269: GitHub advisory CVE-2019-17495Link: GHSA-c427-hjc3-wrfw
Quoting from the disclosure:
LoopBack 3 API Explorer does not suport |
Since these However, the significant downside of doing this would mean that any new vulnerabilities exposed on LB3 doesn't go EOL until Dec 2020, and I still think the best approach would be to update the 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. |
See #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. Related: strongloop/loopback-swagger#65 |
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) 🙂 |
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? |
I can tell you two things:
The UIGiven 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
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 The AuthAs 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. 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). But I said I don't have the solutionI wanted to publish a POC showing the above, but it doesn't work. (401 error). Apparently it's not a matter of setting the In my app (truth be told, the app I developed for a customer) it does work. (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 |
Two more Swagger UI vulns:
These also require crafting custom apispec files, since |
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 |
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 inswagger-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 #254There are two new vulnerabilities reported today, I'm using the following dependencies:
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?
The text was updated successfully, but these errors were encountered: