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

fix(security): address CVE-2021-23358 #1816

Merged

Conversation

zondervancalvez
Copy link
Contributor

@zondervancalvez zondervancalvez commented Jan 27, 2022

fixes: #1775

@zondervancalvez zondervancalvez force-pushed the zondervancalvez/issue1775 branch from c719b34 to ba20b67 Compare January 27, 2022 09:40
@zondervancalvez
Copy link
Contributor Author

review-requested: @petermetz

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zondervancalvez There's no diff though. Did a rebase or merge gone wrong or was this fixed by somebody else concurrently in the meantime and that got merged first with the exact same diff?

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zondervancalvez There's no diff though. Did a rebase or merge gone wrong or was this fixed by somebody else concurrently in the meantime and that got merged first with the exact same diff?

+1

@zondervancalvez zondervancalvez force-pushed the zondervancalvez/issue1775 branch 2 times, most recently from fec71ff to 48b0647 Compare February 24, 2022 06:18
@zondervancalvez
Copy link
Contributor Author

zondervancalvez commented Feb 24, 2022

This is 2 part changes:

  1. All changes are migration from web3-eea to web3js-quorum.
  2. This is also related to fix(security): address CVE-2021-23337 #1820

@zondervancalvez zondervancalvez force-pushed the zondervancalvez/issue1775 branch from 48b0647 to b073e29 Compare February 24, 2022 06:19
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zondervancalvez Question: Where did you get the index.d.ts file from?

@zondervancalvez
Copy link
Contributor Author

@zondervancalvez Question: Where did you get the index.d.ts file from?

Here:
https://github.com/ConsenSys/web3js-quorum/blob/master/src/typescript/index.d.ts

@zondervancalvez zondervancalvez force-pushed the zondervancalvez/issue1775 branch 3 times, most recently from e408284 to 9acbc64 Compare March 8, 2022 06:23
@petermetz petermetz requested review from izuru0 and takeutak March 11, 2022 02:23
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zondervancalvez Please make the tests pass, right now the CI is failing with this:

Error: Cannot find module 'web3-eea'
Require stack:
- /home/runner/work/cactus/cactus/packages/cactus-plugin-ledger-connector-besu/src/test/typescript/integration/plugin-ledger-connector-besu/deploy-contract/private-deploy-contract-from-json-cactus.test.ts
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:889:15)
    at Function.Module._load (internal/modules/cjs/loader.js:745:27)
    at Module.require (internal/modules/cjs/loader.js:961:19)
    at require (internal/modules/cjs/helpers.js:92:18)
    at Object.<anonymous> (/home/runner/work/cactus/cactus/packages/cactus-plugin-ledger-connector-besu/src/test/typescript/integration/plugin-ledger-connector-besu/deploy-contract/private-deploy-contract-from-json-cactus.test.ts:2:966)
    at Module._compile (internal/modules/cjs/loader.js:1072:14)
    at Module.replacementCompile (/home/runner/work/cactus/cactus/node_modules/append-transform/index.js:60:13)
    at Module.m._compile (/home/runner/work/cactus/cactus/node_modules/ts-node/src/index.ts:1311:23)
    at module.exports (/home/runner/work/cactus/cactus/node_modules/default-require-extensions/js.js:7:9)
    at /home/runner/work/cactus/cactus/node_modules/append-transform/index.js:64:4
    at require.extensions.<computed> (/home/runner/work/cactus/cactus/node_modules/ts-node/src/index.ts:1314:12)
    at Object.<anonymous> (/home/runner/work/cactus/cactus/node_modules/append-transform/index.js:64:4)
    at Module.load (internal/modules/cjs/loader.js:937:32)
    at Function.Module._load (internal/modules/cjs/loader.js:778:12)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:76:12)
    at internal/main/run_main_module.js:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/runner/work/cactus/cactus/packages/cactus-plugin-ledger-connector-besu/src/test/typescript/integration/plugin-ledger-connector-besu/deploy-contract/private-deploy-contract-from-json-cactus.test.ts'
  ]
}
# Subtest: ./packages/cactus-plugin-ledger-connector-besu/src/test/typescript/integration/plugin-ledger-connector-besu/deploy-contract/private-deploy-contract-from-json-cactus.test.ts
    1..0 # no tests found
not ok 54 - ./packages/cactus-plugin-ledger-connector-besu/src/test/typescript/integration/plugin-ledger-connector-besu/deploy-contract/private-deploy-contract-from-json-cactus.test.ts # time=11406.73ms

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zondervancalvez Please also resolve the merge conflicts!

@zondervancalvez zondervancalvez force-pushed the zondervancalvez/issue1775 branch 5 times, most recently from d2bf19b to 75f27ed Compare March 15, 2022 02:17
@zondervancalvez zondervancalvez force-pushed the zondervancalvez/issue1775 branch 10 times, most recently from 611a2b1 to 0453a57 Compare March 28, 2022 07:53
@zondervancalvez zondervancalvez force-pushed the zondervancalvez/issue1775 branch from 0453a57 to f756509 Compare March 29, 2022 07:38
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zondervancalvez LGTM, thank you!

fixes: hyperledger-cacti#1775
Signed-off-by: zondervancalvez <zondervan.v.calvez@accenture.com>
@petermetz petermetz force-pushed the zondervancalvez/issue1775 branch from f756509 to 75849db Compare March 29, 2022 23:55
@petermetz petermetz added enhancement New feature or request Quorum Besu dependencies Pull requests that update a dependency file Security Related to existing or potential security vulnerabilities P1 Priority 1: Highest labels Mar 29, 2022
@petermetz
Copy link
Contributor

@zondervancalvez Unless you made some updates to the diff/code that should be explicitly re-checked by the reviewers (because it's different from what was approved), there's usually no need to re-request review from those who already approved (in this case Takuma)

Screenshot from 2022-03-29 16-57-03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Besu dependencies Pull requests that update a dependency file enhancement New feature or request P1 Priority 1: Highest Quorum Security Related to existing or potential security vulnerabilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(security): address CVE-2021-23358
3 participants