DepositPool is susceptible to the inflation attack

[c4-submissions]

Lines of code

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

Vulnerability details

Summary

The DepositPool contract is susceptible to the Inflation Attack, in which the first depositor can be front-runned by an attacker to steal their deposit.

Impact

The DepositPool pool contract acts mainly as a vault: accounts deposit LST assets and get back RSETH tokens (a share) that represents their participation in the vault.

These types of contracts are potentially vulnerable to the inflation attack: an attacker can front-run the initial deposit to the vault to inflate the value of a share and render the front-runned deposit worthless.

The mint amount in the pool is given by the getRsETHAmountToMint() function:

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

095:     function getRsETHAmountToMint(
096:         address asset,
097:         uint256 amount
098:     )
099:         public
100:         view
101:         override
102:         returns (uint256 rsethAmountToMint)
103:     {
104:         // setup oracle contract
105:         address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE);
106:         ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress);
107: 
108:         // calculate rseth amount to mint based on asset amount and asset exchange rate
109:         rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
110:     }

While the getRSETHPrice() is determined by the asset balance in the contracts:

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L52-L79

52:     function getRSETHPrice() external view returns (uint256 rsETHPrice) {
53:         address rsETHTokenAddress = lrtConfig.rsETH();
54:         uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();
55: 
56:         if (rsEthSupply == 0) {
57:             return 1 ether;
58:         }
59: 
60:         uint256 totalETHInPool;
61:         address lrtDepositPoolAddr = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);
62: 
63:         address[] memory supportedAssets = lrtConfig.getSupportedAssetList();
64:         uint256 supportedAssetCount = supportedAssets.length;
65: 
66:         for (uint16 asset_idx; asset_idx < supportedAssetCount;) {
67:             address asset = supportedAssets[asset_idx];
68:             uint256 assetER = getAssetPrice(asset);
69: 
70:             uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
71:             totalETHInPool += totalAssetAmt * assetER;
72: 
73:             unchecked {
74:                 ++asset_idx;
75:             }
76:         }
77: 
78:         return totalETHInPool / rsEthSupply;
79:     }

An attacker can then mint a minimal amount of shares by depositing just 1 token of any valid asset, and then inflate the price per share by donating assets to the deposit pool. Once the share price has been inflated, the front-runned deposit is rounded down to zero or a very small number of shares, causing a loss of funds to the depositor as any precision loss will be captured by the attacker's shares.

Proof of Concept

For simplicity, let's say that the current Chainlink stETH/ETH price is 1 (1e18).

1. An user is about to deposit X amount of stETH in the protocol.
2. The attacker front-runs the transaction and deposits 1 stETH. The attacker is minted (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice() = 1 * 1e18 / 1e18 = 1 RSETH.
3. The attacker donates X amount of stETH tokens to the DepositPool. The stETH balance of the DepositPool is now X + 1.
4. The user transaction is processed. At this point lrtOracle.getRSETHPrice() will be calculated using the sum of balances of stETH (there are no other assets in the deposit pool), (X + 1) * 1e18 / 1 = (X + 1) * 1e18. The minted amount is then calculated as X * 1e18 / (X + 1) * 1e18 = X / (X + 1) = 0. The user is minted 0 RSETH and the deposited amount belongs to the attacker's share.

Recommendation

There are multiple ways to prevent the inflation attack:

There are multiple ways of solving the issue:

1. Introduce a minimum output parameter to control slippage when depositing.
2. Track balances internally so donations cannot affect the share value.
3. Mint an initial amount of "dead shares" so the attacker cannot frontrun and inflate the pool.

A very good discussion of these can be found here.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #42

[c4-judge]

fatherGoose1 changed the severity to 3 (High Risk)

[c4-judge]

fatherGoose1 marked the issue as satisfactory