iglyx - Increasing relayerFee can halt deposit queue

[sherlock-admin]

iglyx

medium

Increasing relayerFee can halt deposit queue

Summary

mintDepositInQueue() doesn't check that queue[i].assets >= relayerFee, leaving it to the minimum deposit check done before.

Vulnerability Detail

If relayer fee be increased while there is a deposit in the queue that was higher amount than old, but lower than new relayer fee, the whole queue will be halted.

Impact

As deposits in the queue will be frozen until relayerFee be manually lowered, some deposits might not be able to get in, which can constitute a loss for the corresponding users if they relied on being invested in the vault, say they do need the corresponding insurance for the next period.

As there is no way to withdraw assets from the queue this is also freeze of funds for all depositors queued (for an arbitrary period, can be permanent if say no owner actions can be taken for any reason).

Code Snippet

mintDepositInQueue() proceeds with queue[i].assets - relayerFee without checks:

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

    uint256 i = length - 1;
    while ((length - _operations) <= i) {
        // this loop impelements FILO (first in last out) stack to reduce gas cost and improve code readability
        // changing it to FIFO (first in first out) would require more code changes and would be more expensive
        _mintShares(
            queue[i].receiver,
            _epochId,
@>          queue[i].assets - relayerFee
        );

But relayerFee can be increased as a part of vault config tuning:

    /** @notice changes relayer fee
     * @param _relayerFee relayer fee
     */
    function changeRelayerFee(uint256 _relayerFee) external onlyFactory {
        relayerFee = _relayerFee;
    }

If this happens while there is a deposit in deposit queue with relayerFee <= queue[i].assets < _relayerFee, then the whole queue will be frozen until changeRelayerFee() be run again setting a lower fee to remedy this.

Tool used

Manual Review

Recommendation

Consider skipping such deposits, returning them on the spot, which looks fine as mintDepositInQueue() is nonReentrant:

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

    while ((length - _operations) <= i) {
+       if (queue[index].assets < relayerFee) {
+           // In a rare case when relayerFee increased, so deposit no longer can be relayed, return it
+           if (queue[index].assets > 0) asset.safeTransfer(queue[index].receiver, queue[index].assets);
+           if (i == 0) break;
+           unchecked { i--; }
+           continue;
+       }        
        // this loop impelements FILO (first in last out) stack to reduce gas cost and improve code readability
        // changing it to FIFO (first in first out) would require more code changes and would be more expensive
        _mintShares(
            queue[i].receiver,
            _epochId,
            queue[i].assets - relayerFee
        );

Duplicate of #1