Mitigation of M-08: Issue not mitigated

[code423n4]

MITIGATION IS NOT CONFIRMED

MITIGATION IS NOT CONFIRMED

Mitigation of M-08: Issue not mitigated

Link to Issue: https://github.com/code-423n4/2023-03-asymmetry-findings/issues/685

Comments

First, there is a clear error in the associated description of mitigation: "Use Chainlink to get rETH". Using Chainlink to obtain the price of Reth has nothing to do with the described issue in M-08.

The issue in M-08 is about a potential timelock that is applied to Reth transfers or burn. Currently this timelock is zero, but if this is eventually reintroduced it will cause a DoS in the protocol as new deposits in the Reth derivative will block unstaking or weight rebalances.

The proposed changeset mitigates the issue, as the Reth deposit implementation is changed to acquire Reth only by swapping it using the Uniswap V3 pool.

However, another related changeset that modifies the deposit() function reintroduces the vulnerability.

Technical Details

The following pull request https://github.com/asymmetryfinance/smart-contracts/pull/228/files is used as a mitigation for M-04. In this changeset, the protocol team removed the Uniswap V3 pool in favor of using RocketSwapRouter.sol.

As we can see in the implementation of the swapTo() function, the router may eventually end up depositing a portion of the amount via Rocket Pool:

https://etherscan.deth.net/address/0x16D5A408e807db8eF7c578279BEeEe6b228f1c1C#code

114:  depositPoolDeposit(depositPool, toDepositPool, msg.sender);

This reintroduces the deposit using the RocketDepositPool contract, which effectively reintroduces the original issue.

Recommendation

Unfortunately, there is no easy way to opt-out from using RocketDepositPool in RocketSwapRouter.sol. A hacky way would be to ensure that the protocol mint rate is below the idealTokensOut variable, so that toDepositPool is always zero, and the deposits are bypassed.

As an alternative, if the intention is to only use Balancer, the balancerSwap() function can be extracted out from RocketSwapRouter.sol and used directly as the implementation of the deposit() function in the Reth derivative.

[elmutt]

known issue. thanks

[c4-judge]

Picodes marked the issue as satisfactory