hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

Circles Protocol contracts
https://aboutcircles.com
GNU Affero General Public License v3.0
0 stars 0 forks source link

Inconsistent Address Initialization of `hub` in `DemurrageCircles` Contract #83

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x87c111e6f030bb2983ea81e3a166ab407f27c9ea72c773b78111b2f17053e8ef Severity: medium

Description: Description\ In the constructor of the DemurrageCircles contract, the hub variable is initialized to address(0x1), as seen here:

hub = IHubV2(address(0x1));

This means that when the contract is first deployed, the hub is not the zero address but is instead set to address(0x1).

In the setup function, there is a check to see if the hub has already been initialized:

if (address(hub) != address(0)) {
    revert CirclesProxyAlreadyInitialized();
}

This check is comparing the hub address against address(0), but in the constructor, the hub is initialized to address(0x1). This causes an inconsistency because after deployment, the hub will already be address(0x1), meaning the setup function would immediately revert, preventing any further initialization.

Impact\ Since the hub is set to address(0x1) in the constructor, the setup function would always fail due to the mismatch in the hub address check. This makes it impossible to properly initialize the contract after deployment. If this issue goes unnoticed, any attempt to set up the contract would fail, potentially halting the system’s operation or requiring a redeployment with a fix.

Recommendation\ We have two options for this. Change the constructor to initialize the hub to address(0) or change the setup function to check if the hub is address(0x1) instead of address(0).

0xmahdirostami commented 1 month ago

As i see it's proxy contract, setup will not revert, it uses proxy storage which is different than DemurrageCircles storage.

benjaminbollen commented 1 month ago

Thank you for your report on the address initialization of hub in the DemurrageCircles Contract. After review, we've determined this is not an issue.

This concern indicates a misunderstanding of the proxy pattern and delegateCalls in our implementation. Our approach is correct and does not result in the scenario you've described.

We appreciate your examination of our contract initialization process. Thank you for your participation in this security review.