StraawHaat - Redundant check in emergencyWithdraw() makes the function almost unusable

[sherlock-admin4]

StraawHaat

Medium

Redundant check in emergencyWithdraw() makes the function almost unusable

Summary

Redundant check in emergencyWithdraw() makes the function almost unusable. This is because the function will almost always cancel the request instead of withdrawing the users' funds:

        if (timestamp > request.deadline) {
            _cancelWithdrawalRequest(sender);
            return actualAmounts;
        }

Vulnerability Detail

The emergencyWithdraw function allows users to withdraw their share of the vault's assets in an emergency situation.

    function emergencyWithdraw(
        uint256[] memory minAmounts,
        uint256 deadline
    )
        external
        nonReentrant
        checkDeadline(deadline)
        returns (uint256[] memory actualAmounts)
    {
        uint256 timestamp = block.timestamp;
        address sender = msg.sender;
        if (!_pendingWithdrawers.contains(sender)) revert InvalidState();
        WithdrawalRequest memory request = _withdrawalRequest[sender];

        if (timestamp > request.deadline) {
            _cancelWithdrawalRequest(sender);
            return actualAmounts;
        }

According to the configuration, 90 days must pass to use this function:

        if (
            request.timestamp + configurator.emergencyWithdrawalDelay() >
            timestamp
        ) revert InvalidState();

require(setup.configurator.emergencyWithdrawalDelay() == 90 days);

But before this check there is a completely unnecessary check which will cancel the request:

        if (timestamp > request.deadline) {
            _cancelWithdrawalRequest(sender);
            return actualAmounts;
        }

This check cancels the withdrawal request if the current timestamp is greater than the request deadline. That is, when a user makes a withdrawal request he chooses by what time(deadline) he wants it to be executed. Almost never it will be more than 90 days.

Any value below configurator.emergencyWithdrawalDelay will enter the above code check and cancel the withdraw request.

This means that if an emergency happens, instead of users withdrawing their funds, the request will be cancelled.

Impact

The function is almost unusable. In the event of an emergency, instead of users saving their funds, the request will be canceled, and they may lose them.

Code Snippet

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

Tool used

Manual Review

Recommendation

Remove the following check:

        if (timestamp > request.deadline) {
            _cancelWithdrawalRequest(sender);
            return actualAmounts;
        }

Duplicate of #57