sherlock-audit / 2024-06-mellow-judging

7 stars 3 forks source link

Angry_Mustache_Man - RestrictingKeeper.sol doesn't have any access control modifier on it #305

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 4 months ago

Angry_Mustache_Man

Medium

RestrictingKeeper.sol doesn't have any access control modifier on it

Summary

RestrictingKeeper.sol doesn't have any access control modifier on it & any Malicious user can use it to rollback updation of important functionalities of the Vault .

Vulnerability Detail

VaultConfigurator.sol follows a 2-stage updation system for the Vault parameters - First the value will be staged then once the configurating delay has been passed it is committed as shown here : https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/VaultConfigurator.sol#L56C2-L80C1


   function _stage(Data storage s, uint256 value) private {
        s.stageTimestamp = block.timestamp;
        s.stagedValue = value;
        bytes32 slot;
        assembly {
            slot := s.slot
        }
        emit Stage(slot, s, value, block.timestamp);
    }

    function _commit(Data storage s, Data storage delay) private {
        uint256 timestamp = s.stageTimestamp;
        if (timestamp == 0) revert InvalidTimestamp();
        if (block.timestamp - timestamp < delay.value)
            revert InvalidTimestamp();
        bytes32 slot;
        assembly {
            slot := s.slot
        }
        emit Commit(slot, s, block.timestamp);
        s.value = s.stagedValue;
        delete s.stageTimestamp;
        delete s.stagedValue;
    }

VaultConfigurator.sol also has one more functionality i.e., rollback which can be used to rollback or delete any staged parameter . As we can see here RestrictingKeeper.sol is given power to rollback https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/utils/RestrictingKeeper.sol#L6C1-L17C2

contract RestrictingKeeper {
    function processConfigurators(
        VaultConfigurator[] memory configurators
    ) external {
        for (uint256 i = 0; i < configurators.length; i++) {
            VaultConfigurator configurator = configurators[i];
            configurator.rollbackStagedBaseDelay();
            configurator.rollbackStagedMaximalTotalSupplyDelay();
            configurator.rollbackStagedMaximalTotalSupply();
        }
    }
}

But it doesn't have any access control modifier , so it can be used by Malicious Actor to rollback the updation of 3 main components _baseDelay , _maximalTotalSupplyDelay & _maximalTotalSupply which are important parameters used by Vault.sol .

Impact

Updation of important parameters can be stopped by Malicious Actor

Code Snippet

https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/VaultConfigurator.sol#L56C2-L80C1

https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/utils/RestrictingKeeper.sol#L6C1-L17C2

Tool used

Manual Review

Recommendation

Give access control modifiers to the function in RestrictingKeeper.sol

Duplicate of #143