Wrong minting of rsETH shares

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L141 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L152 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L109 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L56-L58 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L78

Vulnerability details

Impact

When rsETH is updated using setRSETH function, wrong amount of shares will be minted by the deposit pool. For the first depositor after the update of rsETH address, following check will be true since there is no minted token for new rsETH. So, price returned will be 1 ether and the depositor will get shares equal to their deposit.

if (rsEthSupply == 0) {
            return 1 ether;
        }

In ideal scenario, the shares depositors should get should be less than their deposit amount once vault has enough deposits and yield because vault shares have some yield along with original deposit.

Also, the issue is not limited to the first depositor after the rsETH update. All the subsequent depositors(depositors after rsEthSupply > 0 will get more shares than they should get due to following:

        return totalETHInPool / rsEthSupply;

rsEthSupply for new updated rsEth token may be less than old token. In such cases, vault share will be considered very expensive than it should be. So, it may be the case that rsethAmountToMint maybe zero while their deposit is taken. So, those depositors will lose their deposits without getting any shares(rsETH). In such cases, the first depositors who got the shares can drain the pool since he is the only one who got updated rsETH shares.

rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

Proof of Concept

POC not needed

Tools Used

Manual review

Recommended Mitigation Steps

Remove the mentioned function. If it's necessary, then update the rsETH implementation in such a way that depositors can migrate from old rsETH to new rsETH in 1:1 ratio. Also, even after this, the getRSETHPrice in LRTOracle function should consider rsEthSupply of all the previous rsETH token instead of just the current one.

Assessed type

Math

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #184

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid