Kelp DAO | rsETH - almurhasan's results

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L109

Vulnerability details

##root cause The problem is that a user can know which direction the oracle price of an asset against ETH will move before it does, either through watching the asset price against ETH move in advance of the oracle updating, or watching the mempool for oracle update transactions and frontrunning them.

Proof of Concept

Let’s take example , let assume 1 asset price = 1 ETH, 1 rsETH price = 1 ETH. now the exchange rate of the asset will be updated to 1 asset price = 0.95 ETH. user/MEV can frontrun and let assume users call the function depositAsset with 100 asset amount and mint 100 rsETH (as oracle price is still not updated yet). After the oracle price is updated, the user can backrun(withdraw/redeem his 100 rsETH) because 100 rsETH is now worth 105 asset tokens. So user/mev make a profit of 5 asset tokens.

This can also happen in opposite direction,let assume 1 asset price = 1 ETH, 1 rsETH price = 1 ETH. now the exchange rate of the asset will be updated to 1 asset price = 1.05 ETH. user/MEV(previously deposited) can frontrun and withdraw 100 rsETH (as oracle price is still not updated yet).After the oracle price is updated, the user can backrun and deposit 100 asset tokens, and get 105 rsETH .

Impact

If the deposit/withdrawal amount is big enough,protocol can reach a state of big loss.

Tools Used

manual review

Recommended Mitigation Steps

implementing a 1 hour waiting period on deposit/ withdrawals or
storing deposits and redemptions and executing them at a future exchange rate or creating pause/unpause for deposit/withdraw assets.

Assessed type

MEV

Lines of code

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

Vulnerability details

##summary When the first user enters, totalSupply is set to 1e18.

if (totalSupply == 0) return 1 ether; else return totalETHInPool / rsEthSupply; But the user can immediately withdraw most of his rsETH such that totalSupply << 1e18. The attacker can then increase the totalETHInPool by transferring supported assets to the LRTDepositPool contract.

For subsequent users, the getRSETHPrice will be heavily inflated , The calculation of rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice() can be very inaccurate. In the worst case it rounds down to 0 for users that deposit a value that is less than the value that the attacker transferred in.

Proof of Concept

1.attacker/first depositor mint 10 rsETH and immediately removes all but kept 1 wei. 2.Attacker then directly transfer 10 ETH worth of an asset in LRTDepositPool. 3.When the second user calls the function depositAsset with 5 ETH worth of an asset amount(less than 10 ETH worth of an asset amount), no additional rsETH are minted since rsethAmountToMint is rounded down to 0.

Impact

The first user that stakes can manipulate the total supply of rsETH and by doing so create a rounding error for each subsequent user. In the worst case, an attacker can steal all the funds of the next user.

Tools Used

manual review

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTConfig.sol#L94

Vulnerability details

Proof of Concept

When the manager calls the function updateAssetDepositLimit, there is no check that depositLimit can’t be less than the previous depositLimit .

Tools Used

manual review

Recommended Mitigation Steps

Validate that depositLimit can’t be less than the previous depositLimit when calling the function updateAssetDepositLimit

Assessed type

Invalid Validation