QA Report

[code423n4]

Double Initialize on Diamond Init

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/4dd6149748b635f95460d4c3924c7e3fb6716967/contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol#L36

Vulnerability details

Impact

An attacker could Double Initialize the Diamond Init contract due to unsafe checks.

Proof of Concept

1. Clone the Contest Repo.
2. Start Remixd and go to Remix website and connect with local host
3. Open DiamondInit contract.
4. Compile it and call the init function multiple times with different arguments and accounts.

Tools Used

Remixd and Remix

Root Cause

There is no Standard Initializer modifier on init function. It seems the contract sets the s.initalized = true but this cannot be a valid check because I was able to call it multiple times on testing.

Recommended Mitigation Steps

Use Initializer modifier on Openzepplin library.

[LayneHaber]

connext/monorepo@bf46e73

[0xleastwood]

I disagree with the validity of this finding. init() can be called multiple times, but subsequent calls to this function have no affect on the state of the contract. Hence, I will downgrade this to QA.