code-423n4 / 2024-04-renzo-validation

2 stars 2 forks source link

Potential issues in claim function of WithdrawQueue contract #588

Closed c4-bot-2 closed 4 months ago

c4-bot-2 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L279

Vulnerability details

Impact

The WithdrawQueue:claim function facilitates users in claiming their withdrawal requests. It includes a transfer operation to send the selected redeemed asset to the user. However, if the transfer operation fails, it fails silently, without reverting or stopping the operations, leading to potential contract inconsistencies.

There's a risk of a denial-of-service (DoS) attack if an attacker repeatedly calls the claim function with invalid or non-existent withdrawal request indexes, consuming excessive gas and potentially disrupting the contract's functionality.

Proof of Concept

See the following code:

function claim(uint256 withdrawRequestIndex) external nonReentrant {
        // check if provided withdrawRequest Index is valid
        if (withdrawRequestIndex >= withdrawRequests[msg.sender].length)
            revert InvalidWithdrawIndex();

        WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][
            withdrawRequestIndex
        ];
        if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim();

        // subtract value from claim reserve for claim asset
        claimReserve[_withdrawRequest.collateralToken] -= _withdrawRequest.amountToRedeem;

        // delete the withdraw request
        withdrawRequests[msg.sender][withdrawRequestIndex] = withdrawRequests[msg.sender][
            withdrawRequests[msg.sender].length - 1
        ];
        withdrawRequests[msg.sender].pop();

        // burn ezETH locked for withdraw request
        ezETH.burn(address(this), _withdrawRequest.ezETHLocked);

        // send selected redeem asset to user
        if (_withdrawRequest.collateralToken == IS_NATIVE) {
            payable(msg.sender).transfer(_withdrawRequest.amountToRedeem);
        } else {
            IERC20(_withdrawRequest.collateralToken).transfer(
                msg.sender,
                _withdrawRequest.amountToRedeem
            );
        }
        // emit the event
        emit WithdrawRequestClaimed(_withdrawRequest);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Before processing a withdrawal request, validate the withdrawal request index to ensure its validity and prevent DoS attacks.

Implement proepr error handling mechanisms to handle transfer failures explicitly. Use require or revert statements to revert the transaction and provide meaningful error messages to users in case of transfer failures.

Utilize safe transfer functions, such as SafeERC20, for ERC20 token transfers to ensure that transfers revert in case of failure and provide adequate error feedback.

Assessed type

ETH-Transfer

DadeKuma commented 4 months ago

Transfer reverts on failure. Invalid.

DadeKuma commented 4 months ago

@howlbot reject