NoOne - Missing checks in `whitelistModules`

[sherlock-admin2]

NoOne

medium

Missing checks in whitelistModules

Summary

The whitelistModules function is responsible for whitelisting new modules in the contract. To ensure robustness and security, it is important to include checks for zero addresses and already whitelisted modules

Vulnerability Detail

Impact

Zero Address Check: The initial implementation did not check whether any of the addresses in the beacons_ array were zero addresses. Minting to or interacting with zero addresses is generally considered erroneous and can lead to lost tokens or other unexpected behavior.

Already Whitelisted Module Check: The initial implementation did not check if the module had already been whitelisted. This could result in the same module being added multiple times, leading to redundancy and potential logical errors.

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/abstracts/ArrakisMetaVault.sol#L139

Tool used

Manual Review

Recommendation

function whitelistModules(
    address[] calldata beacons_,
    bytes[] calldata data_
) external onlyOwnerCustom {
    uint256 len = beacons_.length;
    if (len != data_.length) revert ArrayNotSameLength();

    address[] memory modules = new address[](len);
    for (uint256 i; i < len; i++) {
        address beacon = beacons_[i];
        if (beacon == address(0)) revert AddressZero(); // Check for zero address

        address _module = IModuleRegistry(moduleRegistry)
            .createModule(address(this), beacon, data_[i]);

        if (_whitelistedModules.contains(_module)) {
            revert AlreadyWhitelistedModule(_module); // Check for already whitelisted module
        }

        modules[i] = _module;
        _whitelistedModules.add(_module);
    }

    emit LogWhiteListedModules(modules);
}