Skip to content
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

DoS in Axios #268

Closed
tylerlevine opened this issue Apr 25, 2019 · 7 comments · Fixed by #308
Closed

DoS in Axios #268

tylerlevine opened this issue Apr 25, 2019 · 7 comments · Fixed by #308
Labels

Comments

@tylerlevine
Copy link

Describe the bug
A DoS vulnerability for Axios was disclosed publicly yesterday: https://snyk.io/vuln/SNYK-JS-AXIOS-174505

What version are you on?
Tested with stellar-sdk@0.15.3.

To Reproduce

docker run -it --rm node:11 bash
mkdir test
cd test && npm init -y
npm install --save --unsafe-perm stellar-sdk
npm install -g snyk
snyk auth $SNYK_TOKEN
snyk test

Expected behavior
stellar-sdk should not contain known vulnerabilities.

Additional context
Doesn't look like there is a fix for axios available yet. Maybe #241 is a good alternative.

@morleyzhi
Copy link
Contributor

Thanks for the report!

If I'm reading the vuln report correctly, I can't think of a way that a bad actor could use this to attack other people or systems. So that means the issue doesn't have to be resolved urgently.

I think our options are:

  1. Remove axios (See Replace Axios with Fetch #241 )
  2. Wait for axios to close the vuln, and upgrade when it's ready
  3. Ignore the issue entirely

Probably Option 2 is the way to go, unless Option 1 happens first.

If you think of a scenario where it can be exploited, let me know and

@morleyzhi
Copy link
Contributor

Linking the issue and PR so we can track their status from here:

axios/axios#1098

axios/axios#1485

@lirantal
Copy link

lirantal commented May 7, 2019

Hi @morleyzhi 👋

If I may chime in here... not familiar with the Stellar project but if axios is used with Node.js, and access resources that may be user controlled, for example to download a user's profile picture where the user is able to control the URL for it then a malicious user can trigger the Denial of Service vulnerability.

I have detailed more about it here https://snyk.io/blog/a-denial-of-service-vulnerability-discovered-in-the-axios-javascript-package-affecting-all-versions-of-the-popular-http-client/ as well as what you can do to work-around the issue: apply the Snyk patch to published projects that are vulnerable, or catch the error as another mitigation to safely reject it.

I hope it adds some context and happy to help where I can.

@prabhu
Copy link

prabhu commented May 7, 2019

Don't think this is as severe as you put it. For the user profile download example any decent upload profile photo code would have limits in place to limit the file size and format of the file that can be uploaded. There could be other examples but given this is only triggered when there is a call out from the server the chances of exploitation are quite low.

By flagging this as medium snyk and others are breaking a lot of pipelines unnecessarily.

@lirantal
Copy link

lirantal commented May 8, 2019

I didn't say it's severe and Snyk has classified it as medium severity, and I do think it's worth attention and fixing.

Not entirely sure what you mean about limiting the file size or format, or how you think it would help but from the proof-of-concept, if you had this kind of code which does limit the file size of a remote picture that gets downloaded:

require('axios').get(user.profileUrl,
  { maxContentLength: 2000 }
)

and user.profileUrl is an address that a user control then by providing a bigger size than what has been limited in that request will cause a DoS for a Node.js application.

To be more clear, this isn't about a flow where a user uploads a picture and then the server downloads that picture from the server location, but rather it's about the server-side issuing a request to a remote file (any type of file), that the user can control the contents of.

Like I said in the original comment, I'm not familiar with the Stellar project and whether it concerns it or not. I came in to this thread after seeing it was linked from the original Axios issue to see if I can provide more context and help solve it. I'm happy to make the time and run a code review and some user flows to see if there's a vulnerable path in Stellar and help fix it.

@morleyzhi
Copy link
Contributor

@lirantal thanks for the report. The stellar library doesn’t have the functionality you mentioned. We determined that the bug was not exploitable in a profitable way, so we’re okay to wait until the PR gets merged and makes it into an official release.

@lirantal
Copy link

lirantal commented May 8, 2019

Sounds good.
Always available if there's anything I can help with in the future.

morleyzhi pushed a commit that referenced this issue Jun 11, 2019
Updates axios to 0.19.0, which contains a fix for the DoS vulnerability and resolves #268.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants