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

2 stars 2 forks source link

Wrong Return Value of Available To Withdrawal for Non Native Tokens #239

Closed c4-bot-8 closed 6 months ago

c4-bot-8 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L158

Vulnerability details

Impact

  1. Withdrawal Denial of Service when amount to redeem is not actually greater than withdrawBufferTarget at L236 due to wrong implementation of getAvailableToWithdraw(...) function.
  2. Break of normal functionality of getBufferDeficit(...) function at L171 due to wrong implementation of getAvailableToWithdraw(...) function.

    Proof of Concept

    function getAvailableToWithdraw(address _asset) public view returns (uint256) {
        if (_asset != IS_NATIVE) {
    >>>            return IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset];   ❌
        } else {
            return address(this).balance - claimReserve[_asset];
        }
    }

    The function above from the WithdrawQueue contract shows how available to withdraw is derived, it can be noted that the claimed reserve is subtracted from the token balance to determine the available value, the problem is that claimReserve[] is usually stored in the native Eth token value, which means when dealing with non native token as noted from the pointer above, the claimReserve is suppose to be converted first to the specific token involved based on Oracle price, not doing this will return a wrong available balance for non native tokens. This report is not just about the assumption that the ERC20 token used in the contract is of 18 decimal like Eth, but the fact is that the Protocol is assuming the token to Eth value ratio would be 1 : 1 i.e same value But a look at one of the places in the contract where claimReserved is assigned a value at L227-L229 shows that when _assetOut is not a NATIVE token it is converted to its equivalent Eth value at L83-L97 of the oracle contract. So when deriving the non native available value it is right for the claim reserve to be converted back from eth value to the token value before subtraction

    Tools Used

    Manual Review

    Recommended Mitigation Steps

    As adjusted below, the claimReserve should be first converted to the IERC20 value before it is subtracted and returned as the available to withdraw

    function getAvailableToWithdraw(address _asset) public view returns (uint256) {
        if (_asset != IS_NATIVE) {
    +++     uint claimReserveAsset  =  lookupTokenValue(_asset, claimReserve[_asset]);
    ---            return IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset];   
    +++            return IERC20(_asset).balanceOf(address(this)) - claimReserveAsset; 
        } else {
            return address(this).balance - claimReserve[_asset];
        }
    }

Assessed type

ERC20

DadeKuma commented 6 months ago

the problem is that claimReserve[] is usually stored in the native Eth token value

Not true, it's converted to ERC20 balance here:

    uint256 amountToRedeem = renzoOracle.calculateRedeemAmount(
        _amount,
        ezETH.totalSupply(),
        totalTVL
    );

    // update amount in claim asset, if claim asset is not ETH
    if (_assetOut != IS_NATIVE) {
        // Get ERC20 asset equivalent amount
@>      amountToRedeem = renzoOracle.lookupTokenAmountFromValue(
            IERC20(_assetOut),
            amountToRedeem
        );
    }

    // revert if amount to redeem is greater than withdrawBufferTarget
    if (amountToRedeem > getAvailableToWithdraw(_assetOut)) revert NotEnoughWithdrawBuffer();

    // increment the withdrawRequestNonce
    withdrawRequestNonce++;

    // add withdraw request for msg.sender
    withdrawRequests[msg.sender].push(
        WithdrawRequest(
            _assetOut,
            withdrawRequestNonce,
            amountToRedeem,
            _amount,
            block.timestamp
        )
    );

    // add redeem amount to claimReserve of claim asset
@>  claimReserve[_assetOut] += amountToRedeem;

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L253

DadeKuma commented 6 months ago

@howlbot reject