Summary

If the excess balance in the RocketTokenRETH contract is insufficient, the unstake() function could fail, impacting other functions that rely on it, such as unstakeEth() in the BridgeRouterFacet contract.

Vulnerability Details

The unstake() function burns rETH tokens and then attempts to transfer the equivalent amount of ETH. The RocketTokenRETH contract's burn function only allows for excess balance to be withdrawn. If the excess balance in the RocketTokenRETH contract is not sufficient to cover the amount of ETH that needs to be withdrawn, the unstake() function could fail due to insufficient funds.

Impact

This vulnerability could halt the unstaking process and affect the overall functionality of the contract. Users may be unable to unstake their tokens even though they have a sufficient rETH balance. Furthermore, functions that rely on the unstake() function, such as unstakeEth() in the BridgeRouterFacet contract, could also fail, leading to a broader impact on the system's functionality.

Tools Used

Manual review

https://github.com/rocket-pool/rocketpool/blob/967e4d3c32721a84694921751920af313d1467af/contracts/contract/token/RocketTokenRETH.sol#L126-L133

https://github.com/rocket-pool/rocketpool/blob/967e4d3c32721a84694921751920af313d1467af/contracts/contract/deposit/RocketDepositPool.sol#L194-L206

https://github.com/rocket-pool/rocketpool/blob/967e4d3c32721a84694921751920af313d1467af/contracts/contract/deposit/RocketDepositPool.sol#L194-L206

POC

I show in this section how the current withdrawal flow for the Reth derivative is dependend on there being excess balance in the RocketDepositPool.

The current withdrawal flow calls RocketTokenRETH.burn which executes this code:

function burn(uint256 _rethAmount) override external {
// Check rETH amount
require(_rethAmount > 0, "Invalid token burn amount");
require(balanceOf(msg.sender) >= _rethAmount, "Insufficient rETH balance");
// Get ETH amount
uint256 ethAmount = getEthValue(_rethAmount);
// Get & check ETH balance
uint256 ethBalance = getTotalCollateral();
require(ethBalance >= ethAmount, "Insufficient ETH balance for exchange");
// Update balance & supply
_burn(msg.sender, _rethAmount);
// Withdraw ETH from deposit pool if required
withdrawDepositCollateral(ethAmount);
// Transfer ETH to sender
msg.sender.transfer(ethAmount);
// Emit tokens burned event
emit TokensBurned(msg.sender, _rethAmount, ethAmount, block.timestamp);
}

$This executes withdrawDepositCollateral(ethAmount):

function withdrawDepositCollateral(uint256 _ethRequired) private {
// Check rETH contract balance
uint256 ethBalance = address(this).balance;
if (ethBalance >= _ethRequired) { return; }
// Withdraw
RocketDepositPoolInterface rocketDepositPool = RocketDepositPoolInterface(getContractAddress("rocketDepositPool"));
rocketDepositPool.withdrawExcessBalance(_ethRequired.sub(ethBalance));
}

This then calls rocketDepositPool.withdrawExcessBalance(_ethRequired.sub(ethBalance)) to get the ETH from the excess balance:

function withdrawExcessBalance(uint256 _amount) override external onlyThisLatestContract onlyLatestContract("rocketTokenRETH", msg.sender) {
// Load contracts
RocketTokenRETHInterface rocketTokenRETH = RocketTokenRETHInterface(getContractAddress("rocketTokenRETH"));
RocketVaultInterface rocketVault = RocketVaultInterface(getContractAddress("rocketVault"));
// Check amount
require(_amount <= getExcessBalance(), "Insufficient excess balance for withdrawal");
// Withdraw ETH from vault
rocketVault.withdrawEther(_amount);
// Transfer to rETH contract
rocketTokenRETH.depositExcess{value: _amount}();
// Emit excess withdrawn event
emit ExcessWithdrawn(msg.sender, _amount, block.timestamp);
}

And this function reverts if the excess balance is insufficient which you can see in the require(_amount <= getExcessBalance(), "Insufficient excess balance for withdrawal"); check.

Recommendations

The solution for this issue is to have an alternative Unstake mechanism in case the excess balance in the RocketDepositPool is insufficient to handle unstaking.

The alternative mechanism is to sell the rETH tokens via the Uniswap pool.

You can use the RocketDepositPool.getExcessBalance to check if there is sufficient excess ETH to withdraw from Rocketpool or if the withdrawal must be made via Uniswap.

function unstake(uint256 amount) external onlyDiamond {
if (canWithdrawFromRocketPool(amount)) {
RocketTokenRETHInterface rocketETHToken = _getRethContract();
uint256 rethValue = rocketETHToken.getRethValue(amount);
uint256 originalBalance = address(this).balance;
rocketETHToken.burn(rethValue);
uint256 netBalance = address(this).balance - originalBalance;
if (netBalance == 0) revert NetBalanceZero();
(bool sent,) = msg.sender.call{value: netBalance}("");
require(sent, "Failed to send Ether");;
} else {
// Swap rETH for ETH via Uniswap pool
// This part would require additional implementation
}
}

function canWithdrawFromRocketPool(uint256 amount) private view returns (bool) {
IRocketTokenRETH rocketETHToken = _getRethContract();
uint256 ethAmount = rocketETHToken.getEthValue(amount);
IRocketDepositPool rocketDepositPool = IRocketDepositPool(rocketStorage.getAddress(ROCKET_DEPOSIT_POOL_TYPEHASH));
return rocketDepositPool.getExcessBalance() >= ethAmount;
}