search

apigee-127 / swagger-tools

A Node.js and browser module that provides tooling around Swagger.
MIT License
701 stars 373 forks source link

nsp string vulnerability #569

Closed Coosec closed 6 years ago

Coosec commented 6 years ago

There is vulnerability in string package:

swagger-tools@0.10.3 > string@3.3.3 https://nodesecurity.io/advisories/536

I noticed there is a change in master branch related to this vulnerability but when will you publish it on npm?

thomasyuan commented 6 years ago

Same question here. Can't pass CI cause by this issue. When will we release next version?

annyhe commented 6 years ago

Hello! When will the fix be released?

wombat86 commented 6 years ago

Hi! Any new on this?

danyalaytekin commented 6 years ago

Bump, seems quite urgent.

wickedest commented 6 years ago

Please fix urgently - npm install on any package using swagger-tools gets a high security alert.

danyalaytekin commented 6 years ago

Another month passes.

whitlockjc commented 6 years ago

Have you really looked into the scope of the nsp warning? The commit that removes the dependency shows that the usage of stringjs in swagger-tools didn't even use the affected functions that could result in a DoS, and its use was limited to one swagger-tools CLI command: info Chances are good you didn't even know that command existed, let alone actually using it. This, along with the fact that swagger-tools is deprecated, I just didn't see it being of the utmost importance. My time is going into sway and related tooling to completely EOL swagger-tools and the only effort that goes into it must be proven as necessary. Given that I've looked into this, I really didnt' see it being that important since you could just as easily install swagger-tools@master.

I apologize if this comes of snarky. If this is not good enough, feel free to offer to help run the project so things like this can get released.

chrisEff commented 6 years ago

Come on man. I get it, it's a deprecated project and you don't want to put any more effort in it, because you have something shiny and new...BUT: Not everyone can just quickly migrate to sway since it doesn't seem to be backwards compatible to swagger-tools. And even though the vulnerable features from the string package are not actually being used, it's never a nice thing to have a high risk vulnerability alert in your dependencies. And having to do a deep check if you're REALLY affected by this can be very daunting, especially if swagger-tools is not the only one of your dependencies with an alert. And all you have to do to make a lot of people happy is to publish a new version to npm, since the fix is already in master. That's a 5 minute task at max. It probably took you even longer to write that comment. So PLEASE, would you do us that favor?

whitlockjc commented 6 years ago

The point of my response wasn't to force people to sway, since there is no sway-based middleware or CLI yet. When people make snarky remarks to me about a project I run in my own time with zero help elsewhere, I have to quantify the reasoning behind it. All it would take is 5 minutes, or less, to see that this is not an issue and thus the snarky remarks are not warranted.

I get that it's not ideal but at the same time, the sky is not falling either. In this case, there is no impact and so one could easily add an exemption to their project, a very common practice.

I understand your position and I will get a new version out.

whitlockjc commented 6 years ago

swagger-tools@0.10.4 released.