Kelp DAO | rsETH - Phantasmagoria's results

Lines of code

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

Vulnerability details

Impact

In deposit() function _mintRsETH() uses following formula to calculate how much rseth should be minted

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

The problem is that an attacker can manipulate price of rseth and other users when depositng may get 0 rseth.

Proof of Concept

By minting a small amount of rseth and then transferring a large amount of another asset, the attacker can significantly distort the calculated rseth price. Subsequently, when other users attempt to deposit their assets, they won't receive the correct amount of rseth due to the inflated price.

getTotalAssetDeposits() uses balanceOf() to fetch balances of the contracts

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

Tools Used

Manual Review

Recommended Mitigation Steps

Consider not using balanceOf for price calculation

Assessed type

Oracle

Lines of code

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

Vulnerability details

Impact

transferAssetToNodeDelegator() function transfers asset lying in this DepositPool to node delegator contract. To transfer funds manager needs to specify ndcIndex. The problem is that transferAssetToNodeDelegator() doesn't check if delegator with specified index exists. If the manager provides an invalid index that doesn't correspond to an existing node delegator, the funds will be transferred to the address(0), essentially burning the assets.

Proof of Concept

function transferAssetToNodeDelegator(
        uint256 ndcIndex,
        address asset,
        uint256 amount
    )
        external
        nonReentrant
        onlyLRTManager
        onlySupportedAsset(asset)
    {
        address nodeDelegator = nodeDelegatorQueue[ndcIndex];
        if (!IERC20(asset).transfer(nodeDelegator, amount)) {
            revert TokenTransferFailed();
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Add delegator existence check

Assessed type

Invalid Validation