dian.ivanov - Reinitialization risk due to unrestricted function calls in `initializeGlobals` in `ZivoeGlobals`

[sherlock-admin2]

dian.ivanov

medium

Reinitialization risk due to unrestricted function calls in initializeGlobals in ZivoeGlobals

Summary

The initializeGlobals function in the ZivoeGlobals contract, despite documentation comments indicating it should be used only once, can be executed multiple times if the DAO address remains unset (address(0)). This discrepancy between the implementation and the comments may lead to unintended reinitializations of global variables.

Vulnerability Detail

The function includes a requirement that the DAO address must be zero for the function to proceed, which is designed to ensure the function is used only during initial setup. However, if the DAO address is not set during initial (set to zero), the function can be called multiple times. This allows for repeated initialization of all global OTHER than DAO variables, which contradicts the comment specifying single-use.

Impact

The ability to reinitialize critical settings due to the unrestricted calling of initializeGlobals poses a low but tangible risk. It could lead to administrative errors or inconsistencies in the contract's setup, potentially altering operational parameters after deployment. Such functionality contradicts the intended one-time use noted in the comments and could lose trust among users by undermining the perceived stability and security of the contract's configuration.

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/ZivoeGlobals.sol#L179C1-L205C6 :

function initializeGlobals(
    address[] calldata globals,
    address[] calldata stablecoins
) external onlyOwner {
    require(DAO == address(0), "ZivoeGlobals::initializeGlobals() DAO != address(0)");
    // Initialization of global addresses
    DAO     = globals[0];
    // Additional assignments...
    ZVT     = globals[13];
}

Tool used

Manual Review

Recommendation

Implement a "traditional" boolean state variable, such as initialized, that is set to true after the first successful call to initializeGlobals. Modify the require statement to check this variable:

require(!initialized, "ZivoeGlobals::initializeGlobals() already initialized");
initialized = true;

Remove the reliance on the DAO address being zero to trigger initialization. This change will ensure the function behaves as a true initialization function that can only be successfully called once, preventing any future reinitialization unless explicitly handled through different contract logic. Also add zero address checks.

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, function is callable only by trusted admin, who is trusted to do it only once. Besides, it requires DAO == address(0), and since DAO will be set in initialization, it serves as initialization flag as well.