sherlock-audit / 2023-10-mzero-judging

3 stars 2 forks source link

pkqs90 - No incentive for user to call `stopEarning` for M Tokens #47

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

pkqs90

medium

No incentive for user to call stopEarning for M Tokens

Summary

In MToken.sol, there is an EARNERS_LIST. Only addresses on the list may apply for earning interest for holding M tokens. However, whenever a user is taken off the EARNERS_LIST, only the user itself can call stopEarning() to stop itself from earning. There is no incentive for the user itself to do this, as this will only reduce the amount of M tokens he will gain. Which means there is no way to stop an address to earn once it begins earning.

Vulnerability Detail

From the code, we can see only the user itself can stop the user from earning. Since there is no incentive for himself to do so, this function is basically not going be called by anyone. This also means that if the TTG removes a user from EARNERS_LIST, the user would most likely still be earning, and the governence can't do anything about it.

    function stopEarning() external {
        _stopEarning(msg.sender);
    }

    function _stopEarning(address account_) internal {
        MBalance storage mBalance_ = _balances[account_];

        if (!mBalance_.isEarning) return;

        emit StoppedEarning(account_);

        mBalance_.isEarning = false;

        // Treat the raw balance as principal for earner.
        uint112 principalAmount_ = uint112(_balances[account_].rawBalance);

        if (principalAmount_ == 0) return;

        uint240 amount_ = _getPresentAmount(principalAmount_);

        _balances[account_].rawBalance = amount_;

        unchecked {
            totalNonEarningSupply += amount_;
            principalOfTotalEarningSupply -= principalAmount_;
        }

        updateIndex();
    }

Impact

If a user is enabled for earning and starts earning, it is impossible to make it stop earning, unless the user itself wants to do so (which is highly unlikely). The governance can't opt a user out of earning by simply removing it off of the EARNERS_LIST.

Code Snippet

Tool used

Manual review

Recommendation

Allow anyone to call stopEarning(address user), and add a check that the user is 1. currently earning, 2. not on the EARNERS_LIST.

Duplicate of #33

sherlock-admin3 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

seem valid; admin should have a functionality to stop thhe earnigns since the stopearnnig function doesn't have the _revertIfNotApprovedEarner func; medium(2)