I am worried about safety. Any chance to remove dependencies?

[dmitriz]

After the high profile event-stream incident demonstrating the amount of risk to rely on 3rd party dependencies, see e.g.
https://blog.npmjs.org/post/180565383195/details-about-the-event-stream-incident
and
dominictarr/event-stream#116,
I had to look at this library dependencies. Being a major package with 400k downloads weekly, it clearly seems like attractive target for hackers to exploit this vulnerability, which should be a matter of when rather than if, so I was wondering about what it would take to close this vulnerability hole.

At present
https://www.npmjs.com/package/ccxt
lists 3 dependencies:

crypto-js  opencollective  fetch-ponyfill

from which the first 2 were last published 2 years ago (!!) and the last 1 year ago!
The most worrying one is opencollective carrying 6 further dependencies of its own:

chalk inquirer minimist node-fetch babel-polyfill opn

at least one of which is declared deprecated (https://www.npmjs.com/package/opn).
The question here is -- to what extent opencollective is critical and any chance to remove it?

The other dependency, 1 year old package fetch-ponyfill seems just to be a tiny wrapper around node-fetch:

https://github.com/qubyte/fetch-ponyfill/blob/master/fetch-node.js

The node-fetch seems to be actively maintained, so the question is -- any chance to copy-paste the required small code from fetch-ponyfill and replace it with node-fetch?

The last dependency, crypto-js https://www.npmjs.com/package/crypto-js, seem like a major actively downloaded package, which however doesn't seem to be actively maintained being last published 2 years ago. Again, the question is whether it would be feasible to directly import this relatively small library rather than keeping as 2 years old dependency.

[frosty00]

Hi there @dmitriz,

We are really concerned about dependencies since we are a huge target for hackers trying to steal apikeys and secrets. Ideally we would want to have zero dependencies.

The most worrying one is opencollective carrying 6 further dependencies of its own:

Agreed. I can't find any uses of require ('opencollective') in the codebase so hopefully we can remove the dependency and just link to it in the README instead. I already made a PR for crypto-js: #4875 but it has redundant crypto code that we would like to optimize out before merging it.

node-fetch should also be vendored or removed from the lib, to protect us from malicious updates.

[frosty00]

This is a an example of what we do: #4873

By absorbing the code into ccxt they no longer have the permissions to change it, so I made a static_dependencies folder for just that :P

[kroitor]

@frosty00 thx!

@dmitriz Also, check out how package dependency lock files work in npm, PyPI and other packagers:

• https://github.com/ccxt/ccxt/blob/master/package.json#L50
• https://github.com/ccxt/ccxt/blob/master/package-lock.json

By locking those version numbers the entire dep-version-tree is finalized, therefore to hack into the existing version, you will have to break npm or PyPI. Therefore we keep those versions locked as much as we can. And, as @frosty00 noted, we're also including "vendored" code into the lib to lower potential risks.

Let us know if the above doesn't answer the question, basically the short answer is: yes, we have the same concerns, therefore we are working on the safety aspects as well. This lib is MIT, however, so it comes with no warranties to be used at your own risk. You should never rely on someone for checking your security – always make sure yourself. Closing this for now, feel free to reopen it or just ask further questions if you have more.

[dmitriz]

@frosty00

We are really concerned about dependencies since we are a huge target for hackers trying to steal apikeys and secrets. Ideally we would want to have zero dependencies.

Great to hear, thanks!

The most worrying one is opencollective carrying 6 further dependencies of its own:
Agreed.

I can't find any uses of require ('opencollective') in the codebase so hopefully we can remove the dependency and just link to it in the README instead. I already made a PR for crypto-js: #4875 but it has redundant crypto code that we would like to optimize out before merging it.

If it is not even used, it should be even easier to remove.
The crypto-js is prob. the hardest one :)

node-fetch should also be vendored or removed from the lib, to protect us from malicious updates.

Would be great but at least it is currently actively maintained, while the ponyfill is not.
Would the latter be easier to remove sooner?

[dmitriz]

@kroitor

By locking those version numbers the entire dep-version-tree is finalized, therefore to hack into the existing version, you will have to break npm or PyPI. Therefore we keep those versions locked as much as we can. And, as @frosty00 noted, we're also including "vendored" code into the lib to lower potential risks.

Yes, locking versions in the lock files helps too. But it does not help against hidden malicious dependency upgrades whenever the ccxt upgrades the locked version. Not having deps in the first place is still more secure.

Of course, including the code closes that hole, so prob. the best way to go.

In the PR you mentioned the accompanying problem of enlarging the build size of crypto-js.
Which I presume would only occur if the dependency have been already loaded by other libraries. But if ccxt is used as the main communication channel as intended, the user may not have any other libs loading crypto-js, in which case ccxt would load the whole package, which is large as you pointed out. So extracting only the useful part as "static dep" and removing the dep would solve both security and size problems :)

Let us know if the above doesn't answer the question, basically the short answer is: yes, we have the same concerns, therefore we are working on the safety aspects as well. This lib is MIT, however, so it comes with no warranties to be used at your own risk. You should never rely on someone for checking your security – always make sure yourself.

Indeed, checking my own security is why I have started this thread :)

[dmitriz]

I should add, including deps locally and removing the dep is the most secure.
If deps are not removed, they would still be installed and might always run some hidden scripts of their own, so the hole remains.

[dmitriz]

Also note that node-fetch is looking for maintainer and state explicitly that their future status is unclear:
node-fetch/node-fetch#567

[kroitor]

@dmitriz thx for your considerations and comments. BTW there's also a new feature on GitHub that addresses this aspect:
So if you find a big hole in this package someday, I'd try it out ) Feel free to dm me at igor@ccxt.trade if anything.

[frosty00]

So if you find a big hole in this package someday, I'd try it out ) Feel free to dm me at igor@ccxt.trade if anything.

The whole point is that by then we will have all our users funds stolen and our reputation destroyed...

Also paranoid users may be more inclined to use our package if it has no dependencies.

[dmitriz]

@dmitriz thx for your considerations and comments.

You are welcome. :)

BTW there's also a new feature on GitHub that addresses this aspect:

The way I understand, it would only display warning when someone already reported a problem.