code-423n4 / 2023-04-eigenlayer-findings

1 stars 1 forks source link

Assuming a 1-1 peg of Liquid Staked Tokens like stETH and rETH to ETH is dangerous #438

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L164-L173

Vulnerability details

Impact

The price of ETH staking derivatives may not be pegged 1-1 to ETH which affect staking conditions.

Proof of Concept

To stake eth, a user calls depositBeaconChainETH(). The amount parameter is passed into the _addShares function. In _addShares, the amount parameter is switched to shares. delegation.increaseDelegatedShares is called, which calls _delegationReceivedHook and transfers the ETH. Since amount is converted to shares, this assumes that all liquid staking derivatives have the same price, which is not true.

    function depositBeaconChainETH(address staker, uint256 amount)
        external
        onlyEigenPodManager
        onlyWhenNotPaused(PAUSED_DEPOSITS)
        onlyNotFrozen(staker)
        nonReentrant
    {
        // add shares for the enshrined beacon chain ETH strategy
        _addShares(staker, beaconChainETHStrategy, amount);
    }
    function _addShares(address depositor, IStrategy strategy, uint256 shares) internal {
        // sanity checks on inputs
        require(depositor != address(0), "StrategyManager._addShares: depositor cannot be zero address");
        require(shares != 0, "StrategyManager._addShares: shares should not be zero!");

        // if they dont have existing shares of this strategy, add it to their strats
        if (stakerStrategyShares[depositor][strategy] == 0) {
            require(
                stakerStrategyList[depositor].length < MAX_STAKER_STRATEGY_LIST_LENGTH,
                "StrategyManager._addShares: deposit would exceed MAX_STAKER_STRATEGY_LIST_LENGTH"
            );
            stakerStrategyList[depositor].push(strategy);
        }

        // add the returned shares to their existing shares for this strategy
        stakerStrategyShares[depositor][strategy] += shares;

        // if applicable, increase delegated shares accordingly
        delegation.increaseDelegatedShares(depositor, strategy, shares);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Use a price oracle to approximate the rate for converting staked derivatives to ETH or simply use ETH.

Assessed type

Other

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

Sidu28 commented 1 year ago

Seems like a misunderstanding of the protocol. There is no assumption of LSTs being 1-to-1.

c4-sponsor commented 1 year ago

Sidu28 marked the issue as sponsor disputed

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid