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

12 stars 8 forks source link

Lack of slippage control on RestakeManager.deposit() #340

Closed howlbot-integration[bot] closed 5 months ago

howlbot-integration[bot] commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L473

Vulnerability details

Impact

The deposit() function of the RestakeManager.sol contract enables users to deposit assets into the protocol, getting ezETH tokens in return. The function doesn’t have any type of slippage control; this is relevant in the context of the deposit() function, since the amount of tokens received by the user is determined by an interaction with an oracle, meaning that the amount received in return may vary indefinitely while the request is waiting to be executed.

Also the users will have no defense against price manipulations attacks, if they where to be found after the protocol’s deployment.

Proof of Concept

As can be observed by looking at its parameters and implementation, the deposit() function of the RestakeManager.sol contract, doesn’t have any type of slippage protection: RestakeManager.sol#L473

    function deposit(IERC20 _collateralToken, uint256 _amount) external {

Meaning that users have no control over how many ezETH tokens they will get in return for depositing in the contract.

The amount of tokens to be minted, with respect to the deposited amount, is determined by the calculateMintAmount function

RestakeManager.sol#L565-L569

        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );

The values of the tokens being deposited is determined as follows:

RestakeManager.sol#L507

        uint256 collateralTokenValue = renzoOracle.lookupTokenValue(_collateralToken, _amount);

The lookupTokenValue function queries the external oracle for the asset price:

RenzoOracle.sol#L71-L81

    function lookupTokenValue(IERC20 _token, uint256 _balance) public view returns (uint256) {
        AggregatorV3Interface oracle = tokenOracleLookup[_token];
        if (address(oracle) == address(0x0)) revert OracleNotFound();

        (, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
        if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();
        if (price <= 0) revert InvalidOraclePrice();

        // Price is times 10**18 ensure value amount is scaled
        return (uint256(price) * _balance) / SCALE_FACTOR;
    }

Thus, there is no protection for the user in the current implementation. Meaning that the user has no way to predict how many ezETH they will get back at the moment of minting, as the price could be updated while the request is in the mempool.

Tools Used

Manual Review

Recommended Mitigation Steps

An additional parameter could be added to the deposit function, to let users decide the minimum amount of tokens to be received, with a relative check after minting.

Assessed type

Other

c4-judge commented 5 months ago

alcueca marked the issue as satisfactory