[H-02] Owner cannot freeze and thus cannot slash a queued withdraw that has the `delegatedAddress` being the `0` address.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L536-L579 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/Slasher.sol#L115-L121 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/Slasher.sol#L250-L256

Vulnerability details

canSlash checks to see if the block number is less than _whitelistedContractDetails[toBeSlashed][slashingContract], which will be 0 if a user has not delegated an address. This will revert freezeOperatorand not allow an owner/watcher to freeze the address, and thus will not be able to slash a queued withdraw. This means, a user may act maliciously, and then wait the full 7 days before doing a full proper withdraw. In the meantime, they cannot be slashed.

Proof of Concept

Within StrategyManagerUnit.t.sol, add the following lines to the bottom of the script. As we cannot freeze the user, slashQueuedWithdrawal will fail to do its intended job.

    function testFail_attacker_Unstoppable() public {
        uint256 atackAmount = 1000e18;
        IERC20 token = dummyToken;

        //Initialize main strategy
        StrategyBase strategyImplementation = new StrategyBase(strategyManager);
        StrategyBase strategy = StrategyBase(
            address(
                new TransparentUpgradeableProxy(
                    address(strategyImplementation),
                    address(proxyAdmin),
                    abi.encodeWithSelector(StrategyBase.initialize.selector, token, pauserRegistry)
                )
            )
        );

        //get the attacker contract
        Attacker attakerContract = new Attacker(address(strategyManager),address(strategy),address(token));
        deal(address(token), address(attakerContract), 1000e18, true);

        //execute attack
        attakerContract.attack(atackAmount);
        address attacker = address(attakerContract);

        //here we get the params for the slash
        (IStrategyManager.QueuedWithdrawal memory queuedWithdrawal, IERC20[] memory tokensArray,) =
            _setUpQueuedWithdrawalStructSingleStrat(attacker, attacker, token, strategy, atackAmount);
        uint256[] memory indicesToSkip = new uint[](0);

        //strategy owner sees that we did something malicius and tries to slash us
        vm.prank(strategyManager.owner());
        strategyManager.slashQueuedWithdrawal(strategyManager.owner(), queuedWithdrawal, tokensArray, indicesToSkip);
    }

contract Attacker {
    StrategyManager private manager;
    StrategyBase private strategy;
    IERC20 private token;

    constructor(address _manager, address _strategy, address _token) {
        manager = StrategyManager(_manager);
        strategy = StrategyBase(_strategy);
        token = IERC20(_token);
    }

    function attack(uint256 amount) external {
        token.approve(address(manager), amount);

        //get the params we need
        uint256[] memory strategyIndexes = new uint256[](1);
        IStrategy[] memory strategies = new IStrategy[](1);
        uint256[] memory shares = new uint256[](1);
        strategies[0] = strategy;
        strategyIndexes[0] = 0;
        shares[0] = amount;

        manager.depositIntoStrategy(strategy, token, amount);

        //do malicius stuff here

        manager.queueWithdrawal(strategyIndexes, strategies, shares, address(this), true);
    }
}

Recommended Mitigation

Either make it so delegatedAddress = msg.sender upon staking, or alter the canSlash function such that you can freeze the 0 address. Note, if you freeze the 0 address, it will impact all users who have not delegated their shares, and not just the malicious actor.

Assessed type

Invalid Validation

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-sponsor]

Sidu28 marked the issue as sponsor disputed

[Sidu28]

This is by design. Many competitors confused slashing and delays in withdrawals, which may have contributed to this issue being submitted. Stakers who have never delegated should not ever be subject to freezing. Freezing occurs (as an action) on the operator level, and operators are considered delegated to themselves, so the only way a staker can be frozen is if they are also an operator (in which case delegated to themselves) or else delegated to an operator. We will try to improve our docs on this.

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid

[GalloDaSballo]

Closing per the sponsors comment, only delegated shares can be slashed