changeRelayerFee SHOULD BE CONTROLLED BY A TIMELOCK, ELSE USER TRANSACTION COULD BE REVERTED

Summary

In the Carousel.sol contract the changeRelayerFee function is used to change the relayerFee value of the contract. relayerFee is a critical value in the contract since both deposit and depositETH functions are restricted by the minRequiredDeposit(_assets) modifier. This modifier reverts if the depositing assets amount is less than the relayerFee. Hence the asset amount deposited using the deposit or the depositETH functions should be more than the relayerFee.

Vulnerability Detail

So the current fund deposit transactions could be reverted under following conditions

The relayerFee is changed by the CarouselFactory.sol contract by calling the changeRelayerFee function. relayerFee is changed to a higher value than the existing relayerFee value. Above transaction front runs the current transactions which are trying to deposit funds to the contract using the old relayerFee. So when the deposit transactions are executed they are checked against the new relayerFee, even though the deposits were made during the time old relayerFee was existent.

Impact

So when the pending deposit transactions in the mempool are executed they are checked against the new relayerFee and could be reverted if the new relayerFee is more than the amount that the user is depositing. In this case not only the user loses his transaction gas fee but also will be confused as to why his transaction got reverted.

Code Snippet

    function deposit(
        uint256 _id,
        uint256 _assets,
        address _receiver
    )
        public
        override(VaultV2)
        epochIdExists(_id)
        epochHasNotStarted(_id)
        minRequiredDeposit(_assets)
        nonReentrant
    {
     ...
    }

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L87

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L107

    function changeRelayerFee(uint256 _relayerFee) external onlyFactory {
        relayerFee = _relayerFee;
    }

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L602-L604

Tool used

VSCode and Manual Review

Recommendation

Apply a time lock functionality for the changeRelayerFee function, so the relayerFee change will only take place after a delay so user will have enough to time to prepare for the change.

sherlock-audit / 2023-03-Y2K-judging

Udsen - changeRelayerFee SHOULD BE CONTROLLED BY A TIMELOCK, ELSE USER TRANSACTION COULD BE REVERTED #494