DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

unstaking rETH could not work if delay is set again

infect3d

Summary

rETH tokens has implemented an "unstake delay" to prevent sandwich attack. This delay has been set to 0 and replaced by fees, but if for any reason it is set back to a non zero value, users will not be anymore able to unstake from the bridge.

Vulnerability Details

The protocol allow users who deposited rETH into the bridge to either withdraw it or unstake it:

// Exchange system rETH to fulfill zETH obligation to user
function withdraw(address to, uint256 amount)
external
onlyDiamond
returns (uint256)
{
IRocketTokenRETH rocketETHToken = _getRethContract();
// Calculate zETH equivalent value in rETH
uint256 rethValue = rocketETHToken.getRethValue(amount);
// Transfer rETH from this bridge contract
// @dev RETH uses OZ ERC-20, don't need to check success bool
rocketETHToken.transfer(to, rethValue);
return rethValue;
}
​
​
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 thing is, RocketPool rETH tokens have a deposit delay parameter that prevents any user who has recently deposited to transfer or burn tokens:

// https://github.com/rocket-pool/rocketpool/blob/967e4d3c32721a84694921751920af313d1467af/contracts/contract/token/RocketTokenRETH.sol#L156-L172
​
// This is called by the base ERC20 contract before all transfer, mint, and burns
function _beforeTokenTransfer(address from, address, uint256) internal override {
// Don't run check if this is a mint transaction
if (from != address(0)) {
// Check which block the user's last deposit was
bytes32 key = keccak256(abi.encodePacked("user.deposit.block", from));
uint256 lastDepositBlock = getUint(key);
if (lastDepositBlock > 0) {
// Ensure enough blocks have passed
uint256 depositDelay = getUint(keccak256(abi.encodePacked(keccak256("dao.protocol.setting.network"), "network.reth.deposit.delay")));
uint256 blocksPassed = block.number.sub(lastDepositBlock);
require(blocksPassed > depositDelay, "Not enough time has passed since deposit");
// Clear the state as it's no longer necessary to check this until another deposit is made
deleteUint(key);
}
}
}

In the past this delay was set to 5760 blocks mined (aprox. 21h, considering one block per 12s), but it has been set to 0, and replaced by fees instead (the delay was set to mitigate sandwich attack)
While it's not currently possible due to RocketPool's configuration, any future changes made to this delay by the admins could potentially lead to a denial-of-service attack on the unstake() mechanism.

After depositing, a user must wait a delay before being able to withdraw its stake
But here, its the bridge contract who deposit, which mean each time a user deposit through the bridge,
the delay is reset.

As there is a possibility for user to withdraw their rETH directly without unstaking, but still can possibly make it impossible to use the function, I classified it as medium severity.

in the unstake function, rocketTokenRETH.burn is called
Which itself _beforeTokenTransfer, and this one is subject to a delay depending on last deposit

Impact

Users will not be able to unstake from the bridge.

Tools Used

Manual review

Recommended Mitigation Steps

Check that the timelock period will not make revert the withdrawal and, if so, use a DEX to convert the rETH to ETH.

Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-88

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.