search

SmartTokenLabs / token-negotiator

The token-negotiator is an NPM package designed for use with TokenScript.
MIT License
19 stars 11 forks source link

Vulnerable and Outdated Components #253

Closed jot2re closed 2 years ago

jot2re commented 2 years ago

There are dependencies used in the front-end with security issues rated critical.

See section 2.6 in the NFT Minting security report. See Jira issue 291.

nicktaras commented 2 years ago

@oleggrib just let me know if I can help with any of the tasks inside this ticket (from looking in Jira the tasks are currently assigned to you).

jot2re commented 2 years ago

@nicktaras it is just a matter of running npm audit fix and pushing package fixes into production (assuming all tests work afterwards). Also do an extra npm audit afterwards. Because there might be a risk that things don't get auto upgraded if the safe version of the dependency contains api changes.

jot2re commented 2 years ago

The task is mostly about making sure that nothing breaks after upgrading and potentially make some minor api changes if this is needed to upgrade :)

nicktaras commented 2 years ago

@jot2re

Screen Shot 2022-06-30 at 12 42 35 pm

Can I confirm, are these all the updates needed your side as part of the security review.

I'll update these from the screen shot manually rather than force a fix (if possible).

nicktaras commented 2 years ago

From making this suggested change it causes another issue:

1 low severity vulnerability

Insecure Credential Storage in web3 - https://github.com/advisories/GHSA-27v7-qhfv-rqq8

fix available via npm audit fix --force will install web3@1.7.4, which is a breaking change.

Screen Shot 2022-06-30 at 2 48 50 pm

Just looking into how this can be resolved - where at this time its a circular issue, when resolving one vulnerability it fixes the other and then raises the initial as a vulnerability.

jot2re commented 2 years ago

I think it makes sense to try to give it a go to upgrade and then check if the api changes actually are affecting our code. Even if it is, hopefully it should be handleable with minor changes. If it is a big problem, then I think it can be accepted to have a low rated vulnerability.

oleggrib commented 2 years ago

this task related to the https://github.com/TokenScript/token-negotiator/issues/262 so we can change web3 to ethers.js and problem will be solved

nicktaras commented 2 years ago

@micwallace picked this task up for me https://github.com/TokenScript/token-negotiator/pull/275

@jot2re - I'll let you know when we next release for you to be able to see the dependancy issues removed. Thanks.

nicktaras commented 2 years ago

Closing this issue, the solution is now in staging and due to be released soon.

(Will reopen if needed)