Summary
A user should be able at all times to burn his zETH tokens and receive ETH in return. This requires that the rETH held by the protocol can at all times be withdrawn (i.e. converted to ETH). But because withdrawals depend on excess RocketDepositPool
balance, the rETH pool burning could not work, making the inheriting protocol functionality bricked.
Vulnerability Details
Withdrawals are made by calling the RocketTokenRETH.burn function:
Source
function unstake(address to, uint256 amount) external onlyDiamond {
IRocketTokenRETH 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,) = to.call{value: netBalance}("");
assert(sent);
}
The issue with this is that the RocketTokenRETH.burn function only allows for excess balance to be withdrawn. I.e. ETH that has been deposited by stakers but that is not yet staked on the Ethereum beacon chain. So Rocketpool allows users to burn rETH and withdraw ETH as long as the excess balance is sufficient.
Proof of Concept
I show in this section how the current withdrawal flow for the Reth derivative is dependent on there being excess balance in the RocketDepositPool.
The current withdrawal flow calls RocketTokenRETH.burn which executes this code:
Source
function burn(uint256 _rethAmount) override external {
require(_rethAmount > 0, "Invalid token burn amount");
require(balanceOf(msg.sender) >= _rethAmount, "Insufficient rETH balance");
uint256 ethAmount = getEthValue(_rethAmount);
uint256 ethBalance = getTotalCollateral();
require(ethBalance >= ethAmount, "Insufficient ETH balance for exchange");
_burn(msg.sender, _rethAmount);
withdrawDepositCollateral(ethAmount);
msg.sender.transfer(ethAmount);
emit TokensBurned(msg.sender, _rethAmount, ethAmount, block.timestamp);
}
This executes withdrawDepositCollateral(ethAmount):
Source
function withdrawDepositCollateral(uint256 _ethRequired) private {
uint256 ethBalance = address(this).balance;
if (ethBalance >= _ethRequired) { return; }
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:
Source
function withdrawExcessBalance(uint256 _amount) override external onlyThisLatestContract onlyLatestContract("rocketTokenRETH", msg.sender) {
RocketTokenRETHInterface rocketTokenRETH = RocketTokenRETHInterface(getContractAddress("rocketTokenRETH"));
RocketVaultInterface rocketVault = RocketVaultInterface(getContractAddress("rocketVault"));
require(_amount <= getExcessBalance(), "Insufficient excess balance for withdrawal");
rocketVault.withdrawEther(_amount);
rocketTokenRETH.depositExcess{value: _amount}();
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.
Impact
All the ETH swapped for rETH calling BridgeReth::depositEth
would be bricked for rETH, this could lead to a user 'bank run' on DittoETH to not be perjudicated of this protocol externalization to all the depositors.
Since ETH deposits get directly deposited into rETH pool, the only available withdrawing method would be the tokens that are in the bridge, there wouldn't be all the necessary rETH available for all depositors.
Tools Used
Manual review.
Recommendations
Think about altering the Reth bridge to exclusively use the UniswapV3 pool for acquiring rETH. While this approach may result in users receiving slightly less rETH because of slippage, it eliminates potential problems related to deposit delays.
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.
Pseudocode:
function withdraw(uint256 amount) external onlyOwner {
- rocketETHToken.burn(rethValue);
- // solhint-disable-next-line
- (bool sent, ) = address(msg.sender).call{value: address(this).balance}(
- ""
- );
+ if (canWithdrawFromRocketPool(amount)) {
+ rocketETHToken.burn(rethValue);
+ // solhint-disable-next-line
+ } else {
+
+ uint256 minOut = ((((poolPrice() * amount) / 10 ** 18) *
+ ((10 ** 18 - maxSlippage))) / 10 ** 18);
+
+ IWETH(W_ETH_ADDRESS).deposit{value: msg.value}();
+ swapExactInputSingleHop(
+ rethAddress(),
+ W_ETH_ADDRESS,
+ 500,
+ amount,
+ minOut
+ );
+ }
+ // convert WETH into ETH
+ (bool sent, ) = address(msg.sender).call{value: address(this).balance}("");
}
+ function canWithdrawFromRocketPool(uint256 _amount) private view returns (bool) {
+ address rocketDepositPoolAddress = RocketStorageInterface(
+ ROCKET_STORAGE_ADDRESS
+ ).getAddress(
+ keccak256(
+ abi.encodePacked("contract.address", "rocketDepositPool")
+ )
+ );
+ RocketDepositPoolInterface rocketDepositPool = RocketDepositPoolInterface(
+ rocketDepositPoolAddress
+ );
+ uint256 _ethAmount = RocketTokenRETHInterface(rethAddress()).getEthValue(_amount);
+ return rocketDepositPool.getExcessBalance() >= _ethAmount;
+ }
+