trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.31k forks source link

There are security warnings still coming from @ensdomains/ensjs #4601

Open wbt opened 2 years ago

wbt commented 2 years ago

Issue

Due to this underlying issue, @truffle/contract is reporting a High severity npm audit failure. A PR which might fix it was submitted several weeks ago but has not been evaluated or merged, and the last commit to make it into that repo was more than a month prior.

Steps to Reproduce

Run npm audit with @truffle/contract@4.4.2 installed in a project.

Expected Behavior

No audit failures.

Actual Results

Several audit failures, including a high-severity Regex DoS stemming from use of glob-parent <5.1.2, and another high-severity prototype pollution issue from y18n. There are also moderate-severity issues in elliptic < 6.5.3 and in mem < 4.0.0 and in yargs-parser <5.0.0 & 7.0.0 which this should fix as well.

Running npm ls glob-parent gives

`-- @truffle/contract@4.4.2
  `-- @ensdomains/ensjs@2.0.1
    `-- @ensdomains/ens@0.4.3
      `-- ethereumjs-testrpc@6.0.3
        `-- webpack@3.12.0
          `-- watchpack@1.7.5
            +-- chokidar@3.5.2
            | `-- glob-parent@5.1.2
            `-- watchpack-chokidar2@2.0.1
              `-- chokidar@2.1.8
                `-- glob-parent@3.1.0

Running npm ls y18n gives

+-- @truffle/contract@4.4.2
| `-- @ensdomains/ensjs@2.0.1
|   `-- @ensdomains/ens@0.4.3
|     +-- ethereumjs-testrpc@6.0.3
|     | `-- webpack@3.12.0
|     |   `-- yargs@8.0.2
|     |     `-- y18n@3.2.2 deduped
|     +-- ganache-cli@6.12.2
|     | `-- yargs@13.2.4
|     |   `-- y18n@4.0.0
|     `-- solc@0.4.26
|       `-- yargs@4.8.1
|         `-- y18n@3.2.2

There is also a moderate-severity issue in ansi-regex under this path, but that's not the only place it's found, an update would also be needed in chromafi and pulled to Truffle's fork of it.

Response options

1) Assist @ensdomains in addressing their own deprecated dependency 2) Fork the patched dependency and maintain that separately, at least until @ensdomains comes back online 3) Remove the dependency on @ensdomains/ensjs and replace it with something else if needed. 4) Continue to ignore the issue and hope Truffle users generally don't care about security to the extent they'd care about npm audit failures.

In creating this Issue, I propose not #4.

Environment

kevinbluer commented 2 years ago

Hey @wbt, thanks for raising this! We are going to try and get @ensdomains to update the library. If we can't do that we may have to look for alternative implementations.

cc @arachnid @makoto to bring this to your awareness.

Arachnid commented 2 years ago

@makoto Would you mind merging the open PR on ensjs and publishing a new version?

Separately, we'd recommend migrating away from ensjs; ethers has almost all the same functionality, and is better maintained.

haltman-at commented 2 years ago

Unfortunately, as best any of us can tell, the support in ethers for ENS idoesn't allow setting a custom registry address. We could use web3's ENS support, perhaps? That seems to have support for custom registry addresses.

In the meantime, I'll use this as a chance to pester @makoto again about merging https://github.com/ensdomains/ensjs/pull/76 .

Arachnid commented 2 years ago

@haltman-at Please see: https://docs.ethers.io/v5/api/providers/#providers--networks--custom-ens-contract

eggplantzzz commented 2 years ago

Thanks @Arachnid! I must have missed that in the docs.

haltman-at commented 2 years ago

Oh, huh, yeah, I missed that too. That said, having to set that as part of the network is kind of annoying (because we also have to supply the rest of the information, I assume?). But if that really is the problem, well, we can always fall back on web3.js.

TomiOhl commented 2 years ago

Any progress on this? @ensdomains/ens is being deprecated as I see, so probably it's best to solve it from truffle's side?

eggplantzzz commented 2 years ago

There was some initial work done but there is a bit more to be figured out before being able to fully replace it. It is a bit tricky to switch over the functionality that deploys registries/resolvers to test networks.

cliffoo commented 1 year ago

The high severity labeled warning appears to no longer be present on the latest version of @truffle/contract. Closing. Thanks @wbt !

wbt commented 1 year ago

Even in the latest published version of @truffle/contract (4.6.15) I'm still seeing audit warnings (though now of moderate severity) due to this 2020 security advisory, via the ensdomains dependency. It also appears that Truffle's already using the latest published version of @ensdomains/enjs[@2.1.0] prior to the v3 breaking changes.

gnidan commented 1 year ago

Thanks @wbt. Re-opening this to look into it.

eggplantzzz commented 1 year ago

We still have the same difficulties with dealing with replacing this library. Although it looks like they are going to put out a new major version sometime in the future. There are a bunch of 3.0.0-alphaxx versions here.

wbt commented 1 year ago

It may be worth opening a branch to make the breaking changes associated with the latest v3 alpha, to be more ready when v3 is released. The heavy focus on v3 there suggests they may not fix security issues in v2.