howlbot-integration[bot] commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L244-L263

Vulnerability details

Impact

A supported token can be removed by the admin through removeCollateralToken(). It's fair to assume that the action to remove an existing token could be taken in light of some suspicious activity, for example abnormal price manipulation or swings witnessed for the token or some other extreme event. And hence the protocol would want that interactions with the token are cut off.
The removeCollateralToken() function updates the collateralTokens[] array by removing the said _token when called by the role RestakeManagerAdmin. However, existing configurations of withdrawalBufferTarget[_token], tokenOracleLookup[_token] and collateralTokenTvlLimits[_token] are never reset by the protocol unless explicitly called by other admin roles.

This is non-trivial because these functions are supposed to be called by different roles:

removeCollateralToken() and setTokenTvlLimit() have the onlyRestakeManagerAdmin modifier.
setOracleAddress() has the onlyOracleAdmin modifier.
updateWithdrawBufferTarget() has the onlyWithdrawQueueAdmin modifier.

Hence if removeCollateralToken() for _token is called by the RestakeManagerAdmin first, a user can back-run this and can call withdraw() and choose to receive the redemption amount in _token amount. His call will pass the withdrawalBufferTarget check here and the oracle existence check here since they were never automatically reset upon removal of the collateral token.
Given that _token is witnessing suspicious price manipulation activity, the user could potentially extract more value than the protocol intended to provide.

Proof of Concept

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L244-L263

Tools Used

Manual review

Recommended Mitigation Steps

Inside removeCollateralToken(), reset the token's oracle address, the withdraw buffer target as well as the token TVL limit to zero.

Assessed type

Other

jatinj615 commented 5 months ago

In the extreme cases, when protocol decides to remove the collateral Token support from deposit. Then the expected behaviour is to first increase the withdrawBufferTarget to support the deposited collateral in the removed Token and then perform withdrawal from EigenLayer and fill up the withdraw Buffer for users to claim.