0xRajkumar - Attacker Can Harm Other Delegator by Strategic Delegating and UnDelegating

[sherlock-admin2]

0xRajkumar

High

Attacker Can Harm Other Delegator by Strategic Delegating and UnDelegating

Summary and Vulnerability Detail

In L2Staking, we have the delegateStake function, which allows a delegator to delegate their stake to a delegatee. Whenever a delegation is made, we calculate the effectiveEpoch by adding 1 to the currentEpoch. The delegator can also undelegate using undelegateStake, where the effectiveEpoch is calculated the same way as in delegateStake.

An attacker can delegate just 1 second before a new effectiveEpoch starts and then undelegate immediately after the new effectiveEpoch begins, without staying delegated for the entire epoch.

This may seem fair, but it harms other delegators who are staking for the entire epoch to earn rewards.

We also know that Epoch is of one day.

Scenario: The attacker delegates to the delegatee just 1 second before a new epoch starts and then undelegates as soon as the epoch begins, effectively not staying delegated for even 1 day. For example, with 1 second remaining before the new epoch, the attacker delegates and then undelegates right after the epoch starts, essentially delegating for only 1-2 seconds in an entire day.
However, the attacker still receives the same reward for their stake as someone who has been staking for the entire day. The attacker can exploit this by taking out a large loan for 1-2 seconds, effectively harming the rewards of other long-term delegators.

We should note that this happens because we are setting unclaimedEnd to effectiveEpoch - 1 instead of CurrentEpoch - 1 in notifyUndelegation.

    function notifyUndelegation(
        address delegatee,
        address delegator,
        uint256 effectiveEpoch,
        uint256 totalAmount,
        uint256 remainsNumber
    ) public onlyL2StakingContract {
        ...
        unclaimed[delegator].undelegated[delegatee] = true;
        unclaimed[delegator].unclaimedEnd[delegatee] = effectiveEpoch - 1;
    }

Let’s say when the user delegated, the effectiveEpoch was 2, but when they undelegate, the effectiveEpoch is 3. This means the currentEpoch at the time of undelegating is 2, which hasn't been completed yet. However, since we are setting unclaimedEnd to currentEpoch, the user will receive rewards for the current epoch without completing it.

Impact

The attacker can harm other users rewards by taking a large loan, making the impact high.

Tool used

Manual Review

Recommendation

I recommend that we should set unclaimedEnd to currentEpoch - 1.

    function notifyUndelegation(
        address delegatee,
        address delegator,
        uint256 effectiveEpoch,
        uint256 totalAmount,
        uint256 remainsNumber
    ) public onlyL2StakingContract {
        ...
        unclaimed[delegator].undelegated[delegatee] = true;
        unclaimed[delegator].unclaimedEnd[delegatee] = effectiveEpoch - 2; //@note HERE
    }

References

I will share a video by Owen where he explains this category of bugs: https://www.youtube.com/watch?v=-9VmITcdm3c

https://github.com/sherlock-audit/2024-08-morphl2/blob/main/morph/contracts/contracts/l2/staking/Distribute.sol#L112