Kelp DAO | rsETH - zach's results

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L71 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L52

Vulnerability details

Impact The vulnerability allows users to manipulate the rseth price by directly transferring assets to the deposit pool or node delegator contracts. By increasing the totalAssetAmt used in the price calculation, users can artificially inflate the rseth price and potentially earn profits from this manipulation. Proof of Concept For example, let's assume the current asset price is denoted as p. Now, a user transfers an amount x of assets to the deposit pool. User's cost for the transferred assets would be p*x. Before the transfer, the rseth total supply is denoted as t, and the price of rseth is represented as rp. Here is the code rseth price can be inflated.

    function getRSETHPrice() external view returns (uint256 rsETHPrice) {
        address rsETHTokenAddress = lrtConfig.rsETH();
        uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();

        if (rsEthSupply == 0) {
            return 1 ether;
        }
        // ......
        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;
    }

After the transfer, the new rseth price, denoted as rsethPrice_new, can be calculated using the following formula: rsethPrice_new = rp * (1 + (p*x) / t) Here is the code snippet of getAssetDistributionData function. Both assetLyingInDepositPool and assetLyingInNDCs can be manipulated.

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

If the getRSETHPrice() function is used as the rseth price oracle, the user can easily manipulate the rseth price by directly transferring assets to the deposit pool. By transferring a specific amount of assets, the user can influence the calculation and potentially earn profits from the manipulated rseth price.

Recommended Mitigation Steps Use custom storage to store the asset balance instead of simple IERC20.balanceOf

Assessed type

Other

Lines of code

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

Vulnerability details

Impact The vulnerability arises from the addNodeDelegatorContractToQueue function failing to check for duplicated node delegator contracts. This leads to potential issues in the getAssetDistributionData function, where it loops through all node delegators to calculate their asset balance and tokens staked in the eigen layer. If the nodeDelegatorQueue array contains duplicated node delegators, the calculations can be incorrect, ultimately impacting the rseth price and mint amount.

Proof of Concept LRTAdmin may add the same node delegator contract multiple times wrongly to the nodeDelegatorQueue array, creating duplications. Here is the example code:

function test_AddDuplicatedNodeDelegatorToQueue() external{
        vm.startPrank(admin);
        nodeDelegatorQueueProspectives.push(nodeDelegatorContractOne);
        lrtDepositPool.addNodeDelegatorContractToQueue(nodeDelegatorQueueProspectives);
        assertEq(
            lrtDepositPool.nodeDelegatorQueue(0),
            nodeDelegatorQueueProspectives[0],
            "Node delegator index 0 contract is not added"
        );
        assertEq(
            lrtDepositPool.nodeDelegatorQueue(1),
            nodeDelegatorQueueProspectives[1],
            "Node delegator index 1 contract is not added"
        );
        assertEq(
            lrtDepositPool.nodeDelegatorQueue(2),
            nodeDelegatorQueueProspectives[2],
            "Node delegator index 2 contract is not added"
        );
        assertEq(
            lrtDepositPool.nodeDelegatorQueue(3),
            nodeDelegatorQueueProspectives[0],
            "Node delegator index 3 contract is not added"
        );
        vm.stopPrank();
    }

Recommended Mitigation Steps Implement a check in the addNodeDelegatorContractToQueue function to ensure that duplicate node delegator contracts cannot be added to the nodeDelegatorQueue array. Consider using data structures that prevent duplicates, such as sets or mapping with unique keys, to store node delegator contracts.

Assessed type

Other