sherlock-audit / 2024-02-rio-network-core-protocol-judging

4 stars 4 forks source link

peanuts - Leak of value when waiting for assets to be withdrawn after requesting withdrawal #367

Closed sherlock-admin3 closed 8 months ago

sherlock-admin3 commented 8 months ago

peanuts

medium

Leak of value when waiting for assets to be withdrawn after requesting withdrawal

Summary

Vulnerability Detail

Protocol states:

Users will continue to earn yield as they wait for their withdrawal request to be processed.

https://docs.rio.network/rio-architecture/deposits-and-withdraws

The issue here is that users will not earn yield when waiting for their withdrawal request to be processed. Let's look at why this is the case.

When a user withdraws, RioLRTCoordinator.requestWithdrawal() is called. This function will transfer the LRT from the msg.sender to the withdrawalQueue address.

    function requestWithdrawal(address asset, uint256 amountIn) external checkWithdrawal(asset, amountIn) returns (uint256 sharesOwed) {
        // Determine the amount of shares owed to the withdrawer using the current exchange rate.
        sharesOwed = convertToSharesFromRestakingTokens(asset, amountIn);

        // If requesting ETH, reduce the precision of the shares owed to the nearest Gwei,
        // which is the smallest unit of account supported by EigenLayer.
        if (asset == ETH_ADDRESS) sharesOwed = sharesOwed.reducePrecisionToGwei();

        // Pull restaking tokens from the sender to the withdrawal queue.
>       token.safeTransferFrom(msg.sender, address(withdrawalQueue()), amountIn);

These LRT will await the rebalance() call, then it will be burned. This means that the LRT will still be counted under the token.totalSupply which will affect the share calculation of newly minted LRTs.

When rebalance() is called, if there is enough asset in the deposit pool, then the asset will be transferred to the withdrawal queue and the user can withdraw their rewards. The current epoch will then increase by one. If there is not enough asset in the deposit pool, then EigenLayer will be called and the user has to wait a couple of days to receive their assets back.

Note then when waiting for the couple of days, the LRT that is inside the WithdrawalQueue will not be touched. This means that it has the possibility of accruing value. However, this value will not be passed on to the withdrawer as then the withdrawer calls requestWithdrawal(), the current LRT value will already be converted to the asset.

        uint256 availableShares = assetRegistry().convertToSharesFromAsset(asset, assetRegistry().getTotalBalanceForAsset(asset));
        if (sharesOwed > availableShares - withdrawalQueue().getSharesOwedInCurrentEpoch(asset)) {
            revert INSUFFICIENT_SHARES_FOR_WITHDRAWAL();
        }
        withdrawalQueue().queueWithdrawal(msg.sender, asset, sharesOwed, amountIn);

Assume the asset is ETH and the LRT is reETH. Assume that the current exchange is 1 reETH : 1.01 ETH. In seven days (the withdrawal time for EigenLayer), the current exchange rate becomes 1 reETH: 1.02 ETH.

Impact

Users will not earn yield as they wait for their withdrawal request to be processed.

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTCoordinator.sol#L99-L117

Tool used

Manual Review

Recommendation

It's quite difficult to estimate how much the LRT is worth in the future, so if the protocol intends on allowing yield to be earned even during withdrawal, the protocol could recalculate how much a user will receive at the point when the LRT is burned, instead of at the point where the LRT is withdrawn. This will bring about a huge change which may have unexpected consequences.

Alternatively, simply state that withdrawals do not earn yield.

Duplicate of #370