sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

Input Array Lengths in `initializeGlobals` function are not validated #711

Closed sherlock-admin4 closed 6 months ago

sherlock-admin4 commented 6 months ago

Input Array Lengths in initializeGlobals function are not validated

Low/Info issue submitted by dian.ivanov

Summary

The initializeGlobals function in the ZivoeGlobals contract does not perform necessary validations on the lengths of the globals and stablecoins input arrays, leading to potential risks of array out-of-bounds access, which can cause transaction failures.

Vulnerability Detail

The function assumes that the globals array contains at least 14 elements and the stablecoins array contains at least 3 elements without validating these conditions. The function assumes the globals array contains at least 14 elements and the stablecoins array contains at least 3 elements, without validating these conditions. Additionally, the requirement that the DAO address be zero (require(DAO == address(0)) means that once the DAO is initialized to a non-zero address, any subsequent calls to reinitialize other global variables will fail, potentially leaving them unset if not initially configured correctly.

Impact

The impact is low due to the onlyOwner restriction. However, if the DAO address is set and a need arises to adjust other global addresses or tokens due to an earlier incomplete setup, the contract lacks the flexibility to reset these variables, hence a new deployment will be needed with the valid variables set. This could result in administrative challenges or require additional measures like contract upgrades to rectify the setup, leading to potential delays and increased operational complexity.

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)");

        emit TransferredZVL(globals[10]);

        DAO     = globals[0];
        ITO     = globals[1];
        stJTT   = globals[2];
        stSTT   = globals[3];
        stZVE   = globals[4];
        vestZVE = globals[5];
        YDL     = globals[6];
        zJTT    = globals[7];
        zSTT    = globals[8];
        ZVE     = globals[9];
        ZVL     = globals[10];
        GOV     = globals[11];
        TLC     = globals[12];
        ZVT     = globals[13];

        stablecoinWhitelist[stablecoins[0]] = true; // DAI
        stablecoinWhitelist[stablecoins[1]] = true; // USDC
        stablecoinWhitelist[stablecoins[2]] = true; // USDT
    }

Tool used

Manual Review

Recommendation

Implement checks to ensure that the globals array contains at least 14 elements and the stablecoins array contains at least 3 elements before processing them. This can prevent out-of-bounds access and ensure that all expected operations are performed securely and correctly:

require(globals.length == 14, "ZivoeGlobals::initializeGlobals() insufficient number of global addresses");
require(stablecoins.length == 3, "ZivoeGlobals::initializeGlobals() insufficient number of stablecoin addresses");

Also add zero address checks to make sure all addresses are set, because the initialization pattern here strongly depends only on the DAO address being set.

pseudonaut commented 6 months ago

Assumed handled correctly during deployment