hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

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

Initialization Blockage in StandardVault Contract #20

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x808f5b03a7c4763f4cc3fa1321892f51b9229e646a007b98bffbc323f093874b Severity: high

Description: Description\

The StandardVault contract is designed to be used as a master copy in a proxy pattern, where clones of the contract are created and initialized separately. However, the current implementation of the setup function contains a logic flaw that prevents successful initialization of these clones. Specifically, the hub address is initialized to address(1) in the constructor, which is a non-zero address. The setup function checks if the hub address is non-zero before allowing initialization, causing it to always revert with CirclesProxyAlreadyInitialized.

Attack Scenario\

Impact

Recommendation:

To resolve this issue, the initialization logic should be adjusted to allow the setup function to execute successfully for clones:

Modify Initialization Check: Change the condition in the setup function to allow initialization from the placeholder address. For example:

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

Alternative Initialization: Alternatively, initialize hub to address(0) in the constructor. This would require changing the check in setup to:


    constructor() {
    standardTreasury = address(1); // or another placeholder
    hub = IHubV2(address(0)); // Initialize to zero address
}

By implementing these changes, the contract will allow clones to be properly initialized, ensuring that they can interact with a valid hub address and function as intended.

benjaminbollen commented 1 week ago

Thank you for your report on the potential initialization blockage in the StandardVault Contract. After careful review, we've determined that this is not a valid issue.

The concern you've raised indicates a misunderstanding of how the proxy pattern and delegateCalls work in this context. Our implementation is correct and does not result in the self-blocking scenario you've described.

We appreciate your attention to the initialization process of our contracts. While this specific concern isn't applicable, your diligence in examining our system's architecture contributes to the overall security review process. Thank you for your efforts in helping ensure the robustness of our platform.