koxuan - when execute deposit fails, cancel deposit will be called which means that execution fee for keeper will be little for executing the cancellation depending on where the executeDeposit fails

[sherlock-admin]

koxuan

medium

when execute deposit fails, cancel deposit will be called which means that execution fee for keeper will be little for executing the cancellation depending on where the executeDeposit fails

Summary

When execute deposit fails, the deposit will be automatically cancelled. However, since executeDeposit has taken up a portion of the execution fee, execution fee left for cancellation might be little and keeper will lose out on execution fee.

Vulnerability Detail

In executeDeposit when an error is thrown, _handleDepositError is called.

            _handleDepositError(
                key,
                startingGas,
                reasonBytes
            );

Notice that in _handleDepositError that cancelDeposit is called which will pay execution fee to the keeper. However, since the failure can have failed at the late stage of executeDeposit, the execution fee left for the cancellation will be little for the keeper.

    function _handleDepositError(
        bytes32 key,
        uint256 startingGas,
        bytes memory reasonBytes
    ) internal {
        (string memory reason, /* bool hasRevertMessage */) = ErrorUtils.getRevertMessage(reasonBytes);


        bytes4 errorSelector = ErrorUtils.getErrorSelectorFromData(reasonBytes);


        if (OracleUtils.isEmptyPriceError(errorSelector)) {
            ErrorUtils.revertWithCustomError(reasonBytes);
        }


        DepositUtils.cancelDeposit(
            dataStore,
            eventEmitter,
            depositVault,
            key,
            msg.sender,
            startingGas,
            reason,
            reasonBytes
        );
    }
}

Note: This also applies to failed executeWithdrawal.

Impact

Keeper will lose out on execution fee in the event of a failed deposit.

Code Snippet

DepositHandler.sol#L109-L113
DepositHandler.sol#L181-L205

Tool used

Manual Review

Recommendation

Recommend increasing the minimum required execution fee to account for failed deposit and refund the excess to the user when a deposit succeeds.

[xvi10]

this is a valid issue, no code changed, the configuration for gas estimation should be updated to account for this