When user unbonds before transcoder calls reward, then cumulativeRewardFactor for the round is less than it should be

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-livepeer/blob/main/contracts/bonding/BondingManager.sol#L1379 https://github.com/code-423n4/2023-08-livepeer/blob/main/contracts/bonding/libraries/EarningsPoolLIP36.sol#L47-L59

Vulnerability details

Impact

When user unbonds before transcoder calls reward, then cumulativeRewardFactor for the round is less than it should be. As result other delegators loose rewards.

Proof of Concept

Each round transcoder can call reward function in order to claim reward tokens for himself and his delegators. This function can be called at any time in the round. In order to pay correct amount of rewards, protocol stores EarningsPool.Data for each round. One of it's variables is cumulativeRewardFactor, which is used to calculate rewards for user. When reward function is called, then cumulativeRewardFactor for current round is updated.

https://github.com/code-423n4/2023-08-livepeer/blob/main/contracts/bonding/libraries/EarningsPoolLIP36.sol#L47-L59

        function updateCumulativeRewardFactor(
        EarningsPool.Data storage earningsPool,
        EarningsPool.Data memory _prevEarningsPool,
        uint256 _rewards
    ) internal {
        uint256 prevCumulativeRewardFactor = _prevEarningsPool.cumulativeRewardFactor != 0
            ? _prevEarningsPool.cumulativeRewardFactor
            : PreciseMathUtils.percPoints(1, 1);

        earningsPool.cumulativeRewardFactor = prevCumulativeRewardFactor.add(
            PreciseMathUtils.percOf(prevCumulativeRewardFactor, _rewards, earningsPool.totalStake)
        );
    }

As you can see, this function uses earningsPool.totalStake as total amount of staked funds in the epoch.

Now, let's see what happens if user unbonds. In this case decreaseTotalStake function is called, which should decrease staked amount. It will do so only for next round, which means that earningsPoolPerRound for current round will be same. That means that if user unbonds before reward is called, then this unbonded amount is still in the earningsPoolPerRound[currentPeriod].totalStaked, which means that it dilutes cumulativeRewardFactor for all other users.

Example. 1.There is transcoder with delegatedAmount 100 tokens. 2.In round 10 delegator that has 50 tokens unbonds them all. 3.In same round 10 transcoder calls reward and receives 10 more tokens that should be distributed to delegators. 4.cumulativeRewardFactor calculation will use 100 tokens as earningsPool.totalStake. 5.Rest of delegators received and claimed only 5 reward tokens. 6.Unbonded delegator also never received rest 5 tokens. They just stuck in the contract.

Tools Used

VsCode

Recommended Mitigation Steps

In case if user unbonds some amount, before reward call, then update earningsPoolPerRound[currentPeriod].totalStaked for the current round.

Assessed type

Error

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[victorges]

This is also the expected behavior of the protocol, since the stake used to calculate the rewards in a round is, by definition, the stake at the end of the previous round. This snapshotted stake is also referred to as the "active stake".

When the user unbonds, the rewards are minted correctly, with that users stake being taken in consideration as of the end of the previous round. The "unreceived rewards" belong to the delegator that unbonded, which forfeited their rewards by "claiming earnings" (inherent to unbond) before their transcoder calls reward() in the round. This is a well known behavior and duplicate of #90.

[c4-sponsor]

victorges (sponsor) disputed

[c4-judge]

HickupHH3 marked the issue as unsatisfactory: Invalid