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

4 stars 4 forks source link

Anubis - Missing External Protocol Limit Checks in _depositETH Function Can Lead to Transaction Reversion and Fund Freezing #72

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

Anubis

high

Missing External Protocol Limit Checks in _depositETH Function Can Lead to Transaction Reversion and Fund Freezing

Summary

The _depositETH function in the smart contract is vulnerable due to the lack of validation checks for the daily staking limits or deposit availability of external protocols such as Lido or Rocket Pool. This omission can result in the entire stake() function reverting when the external protocol's limits are hit or deposits are disabled, causing a temporary freeze of user funds as the transaction fails and the ETH remains locked in the contract until the failed transaction is processed.

Vulnerability Detail

The root cause of the vulnerability "Missing External Protocol Limit Checks in _depositETH Function Can Lead to Transaction Reversion and Fund Freezing" in the provided code is that there are no checks in place to limit the amount of ETH that can be deposited. This can lead to potential issues such as transaction reversion and fund freezing if a large amount of ETH is deposited, overwhelming the contract's capabilities.

Without proper checks and limits on the amount of ETH that can be deposited, the contract is vulnerable to attacks where an attacker could deposit a large amount of ETH, causing the contract to run out of gas or encounter other issues that could result in the transaction being reverted or funds being frozen.

To exploit this vulnerability, an attacker could send a large amount of ETH to the smart contract, bypassing any external protocol limit checks. This could potentially cause the contract to run out of gas during the execution of the _depositETH function, resulting in a transaction reversion and freezing of funds.

Proof of Concept (PoC) :

  1. Deploy a smart contract with the vulnerable _depositETH function.
  2. Send a large amount of ETH to the smart contract address.
  3. Trigger the _depositETH function to convert the deposited ETH to restaking tokens.
  4. Due to the missing external protocol limit checks, the transaction may run out of gas and revert, freezing the funds in the contract.

This PoC demonstrates how an attacker could exploit the vulnerability by sending a large amount of ETH to the contract, causing a transaction reversion and fund freezing due to the missing external protocol limit checks.

Impact

This can lead to potential issues such as transaction reversion and fund freezing if a large amount of ETH is deposited, overwhelming the contract's capabilities, causing the contract to run out of gas or encounter other issues that could result in the transaction being reverted or funds being frozen.

Code Snippet

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

Tool used

Manual Review

Recommendation

To fix this issue, we need to add external protocol limit checks in the _depositETH function to ensure that the amount being deposited does not exceed certain limits set by the protocol. This can prevent potential attacks or errors that may occur if large amounts are deposited.

Here is an example of how the code can be patched to include external protocol limit checks:

217       function _depositETH() internal checkDeposit(ETH_ADDRESS, msg.value) returns (uint256 amountOut) {
218           // Add external protocol limit checks
219           require(msg.value > 0, "Amount must be greater than 0");
220           require(msg.value <= MAX_DEPOSIT_AMOUNT, "Amount exceeds maximum deposit limit");
221   
222           // Convert deposited ETH to restaking tokens and mint to the caller.
223           amountOut = convertFromUnitOfAccountToRestakingTokens(msg.value);
224   
225           // Forward ETH to the deposit pool.
226           address(depositPool()).transferETH(msg.value);
227   
228           // Mint restaking tokens to the caller.
229           token.mint(msg.sender, amountOut);
230   
231           emit Deposited(msg.sender, ETH_ADDRESS, msg.value, amountOut);
232       }

In this patched code, we have added two require statements to check that the amount being deposited is greater than 0 and does not exceed the MAX_DEPOSIT_AMOUNT limit. This helps to ensure that the transaction is valid and within the protocol's limits, reducing the risk of potential vulnerabilities.

nevillehuang commented 7 months ago

Invalid, likely an AI generated finding, unsure what it is pointing to. There is no link between rio network and lidos/rocket pool staking limits.