BribeRewarder.sol#_modify - Wrong calculation of the rewards per period

Summary

Votes across different periods are accrued for reward calculations instead of kept separately for each period.

Vulnerability Detail

The amounts.update function computes rewards based on the deposited amount and is used in BribeRewarder#_modify as shown below:

(uint256 oldBalance, uint256 newBalance, uint256 oldTotalSupply,) = amounts.update(tokenId, deltaAmount);

If a user deposits 100 tokens for period 1 and 200 tokens for period 2, they should receive rewards for 100 tokens for period 1 and rewards for 200 tokens for period 2.

However, the current implementation adds the newly deposited amount to the previous total deposit amount, which affects the reward calculation.

    function update(Parameter storage amounts, bytes32 key, int256 deltaAmount)
        internal
        returns (uint256 oldAmount, uint256 newAmount, uint256 oldTotalAmount, uint256 newTotalAmount)
    {   
        oldAmount = amounts.amounts[key];
        oldTotalAmount = amounts.totalAmount;

        //if the deltaAmount is 0
        if (deltaAmount == 0) {
            newAmount = oldAmount;
            newTotalAmount = oldTotalAmount;
        } else {
            newAmount = oldAmount.addDelta(deltaAmount);
            newTotalAmount = oldTotalAmount.addDelta(deltaAmount);
            amounts.amounts[key] = newAmount;
            amounts.totalAmount = newTotalAmount;
        }
    }

When rewards are calculated, they are incorrectly based on the total deposited amount of the user, not only of the period deposited amount.

Impact

This issue could cause improper reward distribution, enabling users to obtain more rewards than they are entitled to. This inconsistency can lead to financial losses for the protocol and provide certain users with unfair benefits.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/libraries/Amounts.sol#L68

Tool used

Manual Review

Recommendation

The newAmount parameter should be reset when starting a new period.

Duplicate of #114

sherlock-audit / 2024-06-magicsea-judging

Silvermist - BribeRewarder.sol#_modify - Wrong calculation of the rewards per period #677