Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L52-L79 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L47-L51 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L71-L89 https://github.com/code-423n4/2023-11-kelp/blob/main/src/NodeDelegator.sol#L94-L116

Vulnerability details

Impact

Current implementation of LRTOracle RSETH pricing logic can be manipulated by violating KELP staking flow. It's possible for the malefactor to interact with Eigenlayer contracts ignoring KELP wrappers, and as result destabilize balance of total ETH in LRTDepositPool and RSETH total supply().

Proof of Concept

LRTOracle RSETH pricing calculation includes checking of all assets in the KELP contracts. https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L52-L79

        return totalETHInPool / rsEthSupply;

Underhood it includes three entities: 1) LRTDepositPool itself https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L52-L79

        uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset); // @audit wrapper function to check all assets

2) NodeDelegator https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L71-L89

        assetLyingInDepositPool = IERC20(asset).balanceOf(address(this)); // @audit balance of LRTDepositPool

        uint256 ndcsCount = nodeDelegatorQueue.length;
        for (uint256 i; i < ndcsCount;) {
            assetLyingInNDCs += IERC20(asset).balanceOf(nodeDelegatorQueue[i]); // @audit balance of NodeDelegator
            assetStakedInEigenLayer += INodeDelegator(nodeDelegatorQueue[i]).getAssetBalance(asset);
            unchecked {
                ++i;
            }
        }

3) Strategy that is an entity of EigenLayer https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L121-L124

return IStrategy(strategy).userUnderlyingView(address(this)); // @audit balance of Strategy for address(this) = NodeDelegator

The issue is how Strategy.userUnderlyingView works. Because it's EigenLayer contracts I will use their github links and docuementation. It's transforms shares back into assets, but for this global Strategy variable "totalShares" included for this calculation. https://github.com/Layr-Labs/eigenlayer-contracts/blob/db4506d07b2b9029c31d583d5da6b790484c2b95/src/contracts/strategies/StrategyBase.sol#L249-L251

    function userUnderlyingView(address user) external view virtual returns (uint256) {
        return sharesToUnderlyingView(shares(user));
    }

https://github.com/Layr-Labs/eigenlayer-contracts/blob/db4506d07b2b9029c31d583d5da6b790484c2b95/src/contracts/strategies/StrategyBase.sol#L200-L206

    function sharesToUnderlyingView(uint256 amountShares) public view virtual override returns (uint256) {
        // account for virtual shares and balance
        uint256 virtualTotalShares = totalShares + SHARES_OFFSET;
        uint256 virtualTokenBalance = _tokenBalance() + BALANCE_OFFSET;
        // calculate ratio based on virtual shares and balance, being careful to multiply before dividing
        return (virtualTokenBalance * amountShares) / virtualTotalShares;
    }

Tools Used

Recommended Mitigation Steps

Assessed type

Other

code-423n4 / 2023-11-kelp-findings

LRTOracle RSETH pricing logic exhibits a vulnerability to manipulation. #367