If rewards are distributed before users start staking, a portion of the reward will become permanently stuck

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/UniStaker.sol#L754-L755

Vulnerability details

The UniStaker._checkpointGlobalReward function is used to update the rewardPerTokenAccumulatedCheckpoint and the lastCheckpointTime storage variables.

function _checkpointGlobalReward() internal {
    rewardPerTokenAccumulatedCheckpoint = rewardPerTokenAccumulated();
    lastCheckpointTime = lastTimeRewardDistributed();
}

The issue arises from the fact that the rewardPerTokenAccumulated function returns an old value if totalStaked == 0 so the rewardPerTokenAccumulatedCheckpoint will not be updated in this case. However, the lastTimeRewardDistributed function returns block.timestamp if rewards are streaming at the moment so the lastCheckpointTime value might be updated. It means that if the contract is notified about the reward before the first user stakes, a portion of the reward becomes permanently stuck.

Impact

The portion of the reward will be permanently stuck if the contract is notified about the reward before the first user stakes.

Proof of Concept

The test below illustrates that the rewards streamed before the first user stakes are not distributed to anyone and become inaccessible.

function test_RewardDistributionBeforeStaking() external {
    uint256 amount = 1e18;
    _mintTransferAndNotifyReward(amount);
    
    assertEq(uniStaker.totalStaked(), 0);
    _jumpAhead(uniStaker.REWARD_DURATION()/2);
    
    address depositor = makeAddr("depositor");
    _mintGovToken(depositor, 1);
    
    vm.startPrank(depositor);
    govToken.approve(address(uniStaker), 1);
    uniStaker.stake(1, depositor);
    vm.stopPrank();
    
    assertEq(uniStaker.totalStaked(), 1);
    _jumpAhead(uniStaker.REWARD_DURATION() + 1);
    assertGe(uniStaker.unclaimedReward(depositor) + 1, amount);
}

Tools Used

Manual Review

Recommended Mitigation Steps

Perhaps, the fairest solution is to pause rewards streaming while totalStaked == 0. However, this solution requires additional logic and might be prone to new errors. The easiest way to prevent unnecessary funds loss is to ensure that the first staker will stake UNI tokens right after the contract deploying.

Assessed type

Other

[c4-judge]

MarioPoneder marked the issue as duplicate of #9

[c4-judge]

MarioPoneder marked the issue as duplicate of #369

[c4-judge]

MarioPoneder marked the issue as unsatisfactory:
Invalid