sendwithus / sendwithus_nodejs

Sendwithus NodeJS Client
https://www.sendwithus.com
Apache License 2.0
22 stars 17 forks source link

2 Denial-of-Service vulnerabilities #22

Closed despairblue closed 8 years ago

despairblue commented 8 years ago

Would be cool if restler could be updated to a version that depends on qs >= 1.x

> nsp check
┌───────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────┐
│               │ Denial-of-Service Extended Event Loop Blocking                                                      │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Name          │ qs                                                                                                  │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Installed     │ 0.6.6                                                                                               │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Vulnerable    │ <1.0.0                                                                                              │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Patched       │ >= 1.x                                                                                              │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Path          │ wunderflats-api@0.0.1 > sendwithus@2.9.0 > restler@3.2.2 > qs@0.6.6                                 │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ More Info     │ https://nodesecurity.io/advisories/28                                                               │
└───────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────┐
│               │ Denial-of-Service Memory Exhaustion                                                                 │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Name          │ qs                                                                                                  │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Installed     │ 0.6.6                                                                                               │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Vulnerable    │ <1.0.0                                                                                              │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Patched       │ >= 1.x                                                                                              │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Path          │ wunderflats-api@0.0.1 > sendwithus@2.9.0 > restler@3.2.2 > qs@0.6.6                                 │
├───────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ More Info     │ https://nodesecurity.io/advisories/29                                                               │
└───────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────┘
brandonb927 commented 8 years ago

Hey @despairblue, thanks for this! We'll take a look at seeing what we can do and will update here :)

brandonb927 commented 8 years ago

@despairblue We've noted this PR and we're going to add it to our roadmap to update. We're also considering a client rewrite with a more up-to-date library backing it.

Thanks though, much appreciated! :grinning:

despairblue commented 8 years ago

@brandonb927 thanks guys :+1:

AJCStriker commented 8 years ago

@brandonb927 Just looking to follow up on this to see if any work is being done to fix this?

bvanvugt commented 8 years ago

@AJCStriker @brandonb927 is on vacation this week, but I'll make sure he updates this once he's back. As far as I know, no updates have been made yet.

brandonb927 commented 8 years ago

@AJCStriker is this issue keeping you from using the library? I'd be interested in seeing what we can do to help here if so.

Otherwise, it's looking like a client update isn't in the cards for the immediate future and a client rewrite with a more modern library isn't either. That's not to say it will never be done, but our core focus right now isn't on our client libraries. If you'd like to submit a PR to address the issues, we're open to reviewing it!

Cheers 🍻

AJCStriker commented 8 years ago

Hi @brandonb927,

Sad to hear that repairing this library is not on the priority list for you guys - that is definitely a shame.

For us we can't use any libraries in our products that are on a known blacklist - this package ultimately installs a flawed version of qs which makes our security audit software very unhappy.

Pull Requesting this library to repair these issues is likely out of scope for us as a team right now due to the breadth of the library and our lack of access to certain features for testing.

However we will roll our own for the core functions. I will post a link to that library on this thread when it is complete however it will only use the limited functionality we have access to in our account and what we need to function.

Thanks, Alex

lbstr commented 7 years ago

Could we please revisit this issue?

I've opened this PR which simply updates the version of restler from ~3.2.2 to ~3.4.0. This version of restler pulls in a more recent version of qs that does not have the DoS holes mentioned above.

The tests pass with the update, so if you are confident in your tests, this change seems safe. Let me know if there is anything else I can do to help move this through.

Thank you, Jared

brandonb927 commented 7 years ago

@lbstr thanks for pushing us again on this. We've accepted https://github.com/sendwithus/sendwithus_nodejs/pull/25 as a result and will roll a new release ASAP and push up to npm.

lbstr commented 7 years ago

Thank you @brandonb927 - I'll keep an eye out for the release.