Kelp DAO | rsETH - cryptothemex's results

Lines of code

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

Vulnerability details

Impact

The LRTDepositPool acts as a simplified vault allowing restakers to transfer their liquid staked tokens and receive rsETH tokens based on the current rsETH exchange rate. rsETH are minted to user by interacting with depositAsset function of LRTDepositPool contract and sending amount of liquid staked token (called asset) to it. First, amount of asset is transferred to LRTDepositPool, then rsethAmountToMint is computed and same rsETH are minted to the user. However, there is a flaw in calculation of rsethAmountToMint due to wrong price calculation of rsETH which results in lesser rsETH being minted to user.

This happens due to the fact that while computing the price of rsETH i.e price = totalETHInPool / rsEthSupply, asset deposited by user for minting rsETH is also included in totalETHInPool, whereas, corresponding rsEthSupply has yet not been increased. This, results in biased price of rsETH and lesser rsETH are minted to the user for his deposited asset.

Proof of Concept

getRsETHAmountToMint() function of LRTDepositPool contract returns

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

getRSETHPrice() function of LRTOracle contract returns rsETHPrice as

return totalETHInPool / rsEthSupply; 

where totalETHInPool are computed as sum of all assets held in LRTDepositPool, NDC contract and eigen layer (after adjust price of each asset). Moreover, if rsEthSupply == 0, getRSETHPrice() returns price as 1 ether.

Following explains the PoC working. For simplicity, assume that totalETHInPool is computed by all asset held in LRTDepositPool only (leave NDC contract and eigen layer for the moment).

1. At start, totalETHInPool = 0 and rsEthSupply = 0 as no asset has been deposited yet.
2. Alice calls depositAsset function and transfers 2 rETH to LRTDepositPool
3. Now, rsethAmountToMint = 2 ether as totalETHInPool = 2 and rsEthSupply = 0 (getRSETHPrice() returns price as 1 ether when rsEthSupply == 0).
4. 2 rsETH are minted for Alice. Updated values are totalETHInPool = 2 and rsEthSupply = 2.
5. Now, Bob calls depositAsset function and transfers 99 rETH to LRTDepositPool
6. Now, rsethAmountToMint = 1.96 eth as totalETHInPool = 101 and rsEthSupply = 2 (getRSETHPrice() returns price as 50.5 ether).
7. 1.96 rsETH are minted to Bob. Updated values are totalETHInPool = 101 and rsEthSupply = 3.96.

The above scenarios shows that a user buying later and buying with amount bogger than the asset held by kelp protcol is going to face more biasedness in rsETH price and will get lesser amount of rsETH.

Tests were implemented for above scenario which produces following output (test codes given subsequently)

  rsETH Price: 1000000000000000000
   Alice RSeth Balance: 2000000000000000000
   Pool rEth Balance: 2000000000000000000
  rsETH Supply: 2000000000000000000
  rsETH Price: 50500000000000000000
   Bob RSeth Balance: 1960396039603960396
   Pool rEth Balance: 101000000000000000000
  rsETH Supply: 3960396039603960396

Use following test code in LRTDepositPoolDepositAsset contract to test the above explanation. (also update the oracle to reflect actual RSETH_Price rather than fixed dummy price, code given below).

    function test_DepositAssetCustom() external {
        uint256 balrs; uint256 balr;
        
        address ad = alice;
        vm.startPrank(ad);

        rETH.approve(address(lrtDepositPool), 100 ether);     

        lrtDepositPool.depositAsset(rETHAddress, 2 ether);
        balrs = rseth.balanceOf(address(ad));  
        balr = rETH.balanceOf(address(lrtDepositPool));      
        console.log(" Alice RSeth Balance: %s", balrs);
        console.log(" Pool rEth Balance: %s", balr);
        console.log(" rsETH Supply: %s", rseth.totalSupply());

        vm.stopPrank();

        ad = bob;
        vm.startPrank(ad);

        rETH.approve(address(lrtDepositPool), 100 ether);

        lrtDepositPool.depositAsset(rETHAddress, 99 ether);
        balrs = rseth.balanceOf(address(ad));  
        balr = rETH.balanceOf(address(lrtDepositPool));      
        console.log(" Bob RSeth Balance: %s", balrs);
        console.log(" Pool rEth Balance: %s", balr);
        console.log(" rsETH Supply: %s", rseth.totalSupply());

        vm.stopPrank();
    }

Update LRTOracleMock contract as follows to compute exact value of rsETH by computing deposited assets.

import "forge-std/console.sol";
import { RSETH } from "src/RSETH.sol";
import { ILRTConfig } from "src/interfaces/ILRTConfig.sol";
import { IRSETH } from "src/interfaces/IRSETH.sol";

contract LRTOracleMock {
    function getAssetPrice(address) public pure returns (uint256) {
        return 1e18;
    }

    ILRTConfig lrtConfig;
    MockToken rETH;
    address lrtDepositPool;

    function setAddress(address _lrtConfig, address _rETH, address _lrtDepositPool) external {
        lrtConfig = ILRTConfig(_lrtConfig);
        rETH = MockToken(_rETH);
        lrtDepositPool = _lrtDepositPool;
    }

    function getRSETHPrice() external view returns (uint256) {
        //return 1e18;

        address rsETHTokenAddress = lrtConfig.rsETH();
        uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();

        if (rsEthSupply == 0) {
            console.log("rsETH Price: %s", 1 ether);
            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;
            }
        }*/

        uint256 totalAssetAmt = rETH.balanceOf(lrtDepositPool);
        uint256 assetER = getAssetPrice(address(rETH));

        totalETHInPool += totalAssetAmt * assetER;

        console.log("rsETH Price: %s", totalETHInPool / rsEthSupply);

        return totalETHInPool / rsEthSupply;
    }
}

update setup function of LRTDepositPoolTest contract as follows

    function setUp() public virtual override(RSETHTest, BaseTest) {
        super.setUp();

        // deploy LRTDepositPool
        ProxyAdmin proxyAdmin = new ProxyAdmin();
        LRTDepositPool contractImpl = new LRTDepositPool();
        TransparentUpgradeableProxy contractProxy = new TransparentUpgradeableProxy(
            address(contractImpl),
            address(proxyAdmin),
            ""
        );

        lrtDepositPool = LRTDepositPool(address(contractProxy));

        // initialize RSETH. LRTCOnfig is already initialized in RSETHTest
        rseth.initialize(address(admin), address(lrtConfig));
        vm.startPrank(admin);
        // add rsETH to LRT config
        lrtConfig.setRSETH(address(rseth));
        
        // add oracle to LRT config
        LRTOracleMock oracle = new LRTOracleMock();

        lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(oracle));

        oracle.setAddress(address(lrtConfig), address(rETH), address(lrtDepositPool));

        // add minter role for rseth to lrtDepositPool
        rseth.grantRole(rseth.MINTER_ROLE(), address(lrtDepositPool));

        vm.stopPrank();
    }

Tools Used

Manual Review and Forge Test

Recommended Mitigation Steps

Do not include the asset transferred by user in rsETH price computation for calculating the rsethAmountToMint value. This can be easily avoided by first computing rsethAmountToMint and then asking user to transfer the asset. For example, rewrite line 136-141 as follows :-

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

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

Assessed type

Math