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

4 stars 4 forks source link

pontifex - Inflation attack by assets donation #387

Closed sherlock-admin3 closed 8 months ago

sherlock-admin3 commented 8 months ago

pontifex

medium

Inflation attack by assets donation

Summary

Due to best practice of vault implementations are not used an attacker can make the liquid restaking token exchange rate extreemly big by asset donation to the RioLRTDepositPool contract. This cause roundig errors for the most deposits and zero share minting for small deposits. In some cases attacker can provide a classic first deposit attack.

Vulnerability Detail

The sacrificial deposit is used to prevent the inflation attack. But it is still possible to extreemly increase the liquid restaking token exchange rate by donating in case the MIN_SACRIFICIAL_DEPOSIT is used.

uint256 constant MIN_SACRIFICIAL_DEPOSIT = 1_000;

Though the receive function is used for ETH deposits it is possible to frontrun deploy and predeposit ETH on the RioLRTDepositPool contract. The ERC20 tokens the attack is easier. The attacker can just transfer tokens to the RioLRTDepositPool contract. Also there is a non zero probability that price of the first deposited asset is not sufficient to mint even 1 liquid restaking token. There is no check of the return value from coordinator at the RioLRTIssuer:

    function _deposit(IRioLRTCoordinator coordinator, address asset, uint256 amount) internal {
        if (amount < MIN_SACRIFICIAL_DEPOSIT) revert INSUFFICIENT_SACRIFICIAL_DEPOSIT();
        if (asset == ETH_ADDRESS) {
            if (amount != msg.value) revert INVALID_ETH_PROVIDED();
            coordinator.depositETH{value: amount}();
            return;
        }

        IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
        IERC20(asset).approve(address(coordinator), amount);

        coordinator.deposit(asset, amount);
    }

There are no checks for zero amountOut at the RioLRTCoordinator:

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

        // Pull tokens from the sender to the deposit pool.
        IERC20(asset).safeTransferFrom(msg.sender, address(depositPool()), amountIn);

        // Mint restaking tokens to the caller.
        token.mint(msg.sender, amountOut);

        emit Deposited(msg.sender, asset, amountIn, amountOut);
    }

The balance of the RioLRTDepositPool is used to calculate TVL.

All these aspects make the attack posible. In case of 10 * 18 anmount donation all deposits less than 0.0001 10 ** 18 will be left.

Impact

Assets losses, unintendent behavior of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTIssuer.sol#L163-L175 https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTAssetRegistry.sol#L89-L102 https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTAssetRegistry.sol#L188-L196

Tool used

Manual Review

Recommendation

Consider using best practices to prevent an inflation attack. Also check the amount of tokens received for the sacrificial deposit.

Duplicate of #218