code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

Users Cannot Freely Redeem rsETH for the Underlying Asset #496

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/RSETH.sol#L54-L56 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L74-L89 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L183-L197

Vulnerability details

Impact

Users deposit the Underlying Asset in return for rsETH which generates yield via different strategies. When a user redeems rsETH, the user should receive the underlying asset in return. However, the burnFrom() function of rsETH is controlled by a privileged role, and when the transferBackToLRTDepositPool() function is called in the NodeDelegator function, the underlying asset gets transferred to the LRTDepositPool contract and there's no way for the user to redeem the underlying asset from the LRTDepositPool contract. In fact, the only way that the underlying asset leaves the LRTDepositPool contract is via the transferAssetToNodeDelegator() function which is not intended for user redemption. Users deposit is a one way street.

Proof of Concept

The following code shows that the rsETH token burning is controlled by a privileged role

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/RSETH.sol#L54-L56

The following function is used to transfer underlying asset from NodeDelegator back to the LRTDepositPool contract

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L74-L89

The only way that the underlying asset can be transferred out of the LRTDepositPool contract is via the function below, which is intended to deposit the asset to a strategy via the NodeDelegator contract, and not intended for user redemption of the underlying asset. There's no function in this LRTDepositPool contract that is used for user redemption.

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L183-L197

Tools Used

Manual Review

Recommended Mitigation Steps

There should be a function added in the LRTDepositPool contract so users can redeem the underlying asset when they burn the rsETH token they own. The privileged role that controls rsETH token burning should belong to the LRTDepositPool contract to facilitate the above.

Assessed type

Access Control

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #43

c4-judge commented 11 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid