sherlock-audit / 2024-06-mellow-judging

7 stars 3 forks source link

Audinarey - Missing reentrancy guard in the `delegateCall(...)` function #311

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 4 months ago

Audinarey

High

Missing reentrancy guard in the delegateCall(...) function

Summary

The delegateCall(...) function is used to delegate calls to a specified address with given data.

Vulnerability Detail

The problem is if the called address has at least an OPERATOR role, it can reenter the delegateCall(...) function with its own parameters due to missing nonReentrant modifier.

    function delegateCall(
        address to,
        bytes calldata data
    ) external returns (bool success, bytes memory response) {
        _requireAtLeastOperator();
        if (!configurator.isDelegateModuleApproved(to)) revert Forbidden();
        IValidator validator = IValidator(configurator.validator());
        validator.validate(
            msg.sender,
            address(this),
            abi.encodeWithSelector(msg.sig, to, data)
        );
        validator.validate(address(this), to, data);
        (success, response) = to.delegatecall(data); // @audi tif to is an operator, he can reenter the delegateCall
        emit DelegateCall(to, data, success, response);
    }

Impact

the delegateCall(...) function can be reentered

Code Snippet

https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/Vault.sol#L267-L280

Tool used

Manual Review

Recommendation

Add the nonReentrant modifier to the delegateCall(...) function as shown below

    function delegateCall(
        address to,
        bytes calldata data

  - ) external returns (bool success, bytes memory response) {
  + ) external nonReentrant returns (bool success, bytes memory response) {

        _requireAtLeastOperator();
        if (!configurator.isDelegateModuleApproved(to)) revert Forbidden();
        IValidator validator = IValidator(configurator.validator());
        validator.validate(
            msg.sender,
            address(this),
            abi.encodeWithSelector(msg.sig, to, data)
        );
        validator.validate(address(this), to, data);
        (success, response) = to.delegatecall(data); // @audi tif to is an operator, he can reenter the delegateCall
        emit DelegateCall(to, data, success, response);
    }