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

2 stars 2 forks source link

no check for erc20 withdraw buffer deficit #911

Closed c4-bot-8 closed 5 months ago

c4-bot-8 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L134-L144 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L491-L576

Vulnerability details

Impact

A similar logic to the _checkAndFillETHWithdrawBuffer logic was not implemented for the ERC20 token deposit. This means that the withdrawalBufferTarget through the fillERC20withdrawBuffer can be exceeded.

This could have a big impact on the system as there is a possibility of a large imbalance between the value of the depositQueue contract which does the restaking on the restakingManager contract. There could be a dearth of value on deposits if a large number of ERC20 ETH derivatives were deposited through the deposit function in the restakeManager contract and it takes 7 days before a withdrawal is claimed.

While there is an implementation of bufferFill in the restakeManager contract, the original impact is not solved which is having some value of ETH for staking in the Operator Delegate through the reStakingManager, instead the ERC20 tokens are sent to the OD.

Proof of Concept

The implementation for erc20 withdraw buffer can be found here

function fillERC20withdrawBuffer(
        address _asset,
        uint256 _amount
    ) external nonReentrant onlyRestakeManager {
        if (_amount == 0 || _asset == address(0)) revert InvalidZeroInput();
        // safeTransfer from restake manager to this address
        IERC20(_asset).safeTransferFrom(msg.sender, address(this), _amount);
        // approve the token amount for withdraw queue
        IERC20(_asset).safeApprove(address(withdrawQueue), _amount);
        // call the withdraw queue to fill up the buffer
        withdrawQueue.fillERC20WithdrawBuffer(_asset, _amount);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

implement the similar swap system in the WithdrawQueue to send the needed deficit and the rest of the ERC20 value is converted to ETH for ETH restaking.

Assessed type

Other

raymondfam commented 5 months ago

@howlbot reject

raymondfam commented 5 months ago

Buffer has been filled before this function is called from OD and restakeManager. It's sweepERC20() that's missing to refill check.