sherlock-audit / 2024-06-magicsea-judging

2 stars 0 forks source link

neogranicen - Wrong reward token calculation in MasterChefV2 Contract #646

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 2 months ago

neogranicen

Medium

Wrong reward token calculation in MasterChefV2 Contract

Summary

Before setting a new reward per second rate in masterchefV2 the pools are not updated making it possible for pools to recieve morReviewe or less then they deserve depending on wether the rate was increased or decreased

Vulnerability Detail

Althou the team was going to added the function to update pools before they commented it out and added the following comment so its safe to assume that its not present in the code:

    /**
     * @dev Sets the LUM per second.
     * It will update all the farms that are in the top pool IDs.
     * @param lumPerSecond The new LUM per second.
     */
    function setLumPerSecond(uint96 lumPerSecond) external override onlyOwner {
        if (lumPerSecond > Constants.MAX_LUM_PER_SECOND) revert MasterChef__InvalidLumPerSecond();
        // _updateAll(_voter.getTopPoolIds()); // todo remove this
        _lumPerSecond = lumPerSecond;
        emit LumPerSecondSet(lumPerSecond);
    }

The following 2 senario illusrates the Vunrability best:

Senario 1:

Senario 2:

Impact

Because of the issue outilned above users are going to loose rewards both when the rate decreases and when it increases for last claimers.

The degree of damage will depend on how frequently rewards are updated and the average activity on the pools in the masterchef

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MasterchefV2.sol#L352

Tool used

Manual Analysis

Recommendation

Add the _updateAll function back

Duplicate of #177