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

4 stars 4 forks source link

merlin - No slippage protection is implemented for the deposit and depositETH functions in the RioLRTCoordinator #342

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

merlin

medium

No slippage protection is implemented for the deposit and depositETH functions in the RioLRTCoordinator

Summary

Users may receive fewer restaking tokens than expected because there is no amountOutMin protection implemented.

Vulnerability Detail

Before depositing the underlying asset, the user knows how many restaking tokens they will receive using the RioLRTCoordinator.convertFromAssetToRestakingTokens function. Afterward, they call the deposit function, which depends on the total value of all underlying assets in the unit of account, obtained from the getTVL function in the RioLRTAssetRegistry smart contract.

function deposit(address asset, uint256 amountIn) external checkDeposit(asset, amountIn) returns (uint256 amountOut) {
        // Convert deposited asset amount to restaking tokens.
-->     amountOut = convertFromAssetToRestakingTokens(asset, amountIn);

        // code
    }
--------------------------------    
function convertFromAssetToRestakingTokens(address asset, uint256 amount) public view returns (uint256) {
        uint256 value = assetRegistry().convertToUnitOfAccountFromAsset(asset, amount);
-->     return convertFromUnitOfAccountToRestakingTokens(value);
    }
--------------------------------    

function convertFromUnitOfAccountToRestakingTokens(uint256 value) public view returns (uint256) {
        ///code
-->     return value * supply / tvl;
    }               

So if a user wants to buy restaking tokens for tokenA, but tokenB's price changes drastically and crosses the Chainlink Deviation Threshold, its price will change, affecting the TVL. This can lead to a change in the number of restaking tokens the user receives.

Impact

There is no slippage control on the deposit() and depositETH() functions of RioLRTCoordinator, which exposes users to receiving fewer restaking tokens than expected.

Code Snippet

contracts/restaking/RioLRTCoordinator.sol#L79 contracts/restaking/RioLRTCoordinator.sol#L163 contracts/restaking/RioLRTAssetRegistry.sol#L188-L196

Tool used

Manual Review

Recommendation

Consider adding a minAmountOut argument in deposit() and depositETH() functions.

Duplicate of #214