0xMAKEOUTHILL - Insufficient validation in `onModify` function

[sherlock-admin3]

0xMAKEOUTHILL

High

Insufficient validation in onModify function

Summary

Insufficient validation in MasterChefRewarder's onModify allows for user to arbitrary change balance of another user without permission.

Vulnerability Detail

    /**
     * @dev Called by the caller contract to update the rewards for a given account.
     * If the rewarder is not linked, the function will revert.
     * @param account The account to update rewards for.
     * @param pid The pool ID of the staking pool.
     * @param oldBalance The old balance of the account.
     * @param newBalance The new balance of the account.
     * @param oldTotalSupply The old total supply of the staking pool.
     * @return reward The amount of rewards sent to the account.
     */
function onModify(address account, uint256 pid, uint256 oldBalance, uint256 newBalance, uint256 oldTotalSupply)
        public
        override(BaseRewarder, IBaseRewarder)
        returns (uint256 reward)
    {

        if (_status != Status.Linked) revert MasterChefRewarder__NotLinked();

        reward = BaseRewarder.onModify(account, pid, oldBalance, newBalance, oldTotalSupply);

        _claim(account, reward);
    }

By the comments above you can see that the function is supposed to be called by the caller, but since no validation is added anyone can call the function onModify with arbitrary params.

Which then calls BaseRewarder's OnModify:

function onModify(address account, uint256 pid, uint256 oldBalance, uint256 newBalance, uint256 oldTotalSupply)
        public
        virtual
        override
        returns (uint256)
    {
        if (msg.sender != _caller) revert BaseRewarder__InvalidCaller();
        if (pid != _pid()) revert BaseRewarder__InvalidPid(pid);

        return _update(account, oldBalance, newBalance, oldTotalSupply);
    }

The check for msg.sender != caller will pass since the msg.sender will be the MasterChefRewarder and then _update is called:

function _update(address account, uint256 oldBalance, uint256 newBalance, uint256 oldTotalSupply)
        internal
        virtual
        returns (uint256 rewards)
    {

        uint256 totalUnclaimedRewards = _totalUnclaimedRewards;

        uint256 reserve = _reserve;

        uint256 totalRewards = oldTotalSupply == 0 ? 0 : _rewarder.getTotalRewards(_rewardsPerSecond, _endTimestamp, oldTotalSupply);

        rewards = _rewarder.update(account, oldBalance, newBalance, oldTotalSupply, totalRewards);

        _totalUnclaimedRewards = totalUnclaimedRewards + totalRewards - rewards;

        _reserve = reserve - rewards;

    }

And here with arbitrary params set by the user calling the function he can decrease\increase rewards and balances for particular user.

Impact

Loss of funds for the protocol

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/MasterChefRewarder.sol#L68-L78

Tool used

Manual Review

Recommendation

Add the validation for the caller of the function

[0xSmartContract]

This issue is invalid according to Sherlock's Criteria for the following reasons:

It falls into the category of gas optimizations, which is considered invalid. Lack of user input validation is considered an invalid issue.