Rewards distribution can be disrupted by a early user

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-yield/blob/e946f40239b33812e54fafc700eb2298df1a2579/contracts/ConvexStakingWrapper.sol#L206-L224

function _calcRewardIntegral(
    uint256 _index,
    address[2] memory _accounts,
    uint256[2] memory _balances,
    uint256 _supply,
    bool _isClaim
) internal {
    RewardType storage reward = rewards[_index];

    uint256 rewardIntegral = reward.reward_integral;
    uint256 rewardRemaining = reward.reward_remaining;

    //get difference in balance and remaining rewards
    //getReward is unguarded so we use reward_remaining to keep track of how much was actually claimed
    uint256 bal = IERC20(reward.reward_token).balanceOf(address(this));
    if (_supply > 0 && (bal - rewardRemaining) > 0) {
        rewardIntegral = uint128(rewardIntegral) + uint128(((bal - rewardRemaining) * 1e20) / _supply);
        reward.reward_integral = uint128(rewardIntegral);
    }

reward.reward_integral is uint128, if a early user mint (wrap) just 1 Wei of convexToken, and make _supply == 1, and then tranferring 5e18 of reward_token to the contract.

As a result, reward.reward_integral can exceed type(uint128).max and overflow, causing the rewards distribution to be disrupted.

Recommendation

Consider wrap a certain amount of initial totalSupply, e.g. 1e8, and never burn it. And consider using uint256 instead of uint128 for reward.reward_integral. Also, consdier lower 1e20 down to 1e12.

[alcueca]

Assets are not a direct risk if it is the first user disrupting the contract. We would need to redeploy better code, but that's it. I suggest this is reported as medium, as merits for a DoS attack.

As suggested elsewhere, the right solution if uint128 is to be used in storage, is to cast up to uint256 for calculations, and then back to uint128 to store again.

[GalloDaSballo]

The warden identified a way an early depositor can grief the system, I believe the finding to be valid, and because:

• It would conditionally disrupt the system
• It would "brick the contract" at the loss of the griefer
• No additional funds would be at risk

I believe medium severity to be appropriate