A malicious frontrunner can make the `Mutebond` contract broken when the owner decreases `maxPayout`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-mute/blob/4d8b13add2907b17ac14627cfa04e0c3cc9a2bed/contracts/bonds/MuteBond.sol#L119

Vulnerability details

Impact

The Mutebond contract might stop working after the owner decreased maxPayout by a malicious frontrunner.

Proof of Concept

setMaxPayout() can be used to reset maxPayout.

    function setMaxPayout(uint _payout) external {
        require(msg.sender == customTreasury.owner());
        maxPayout = _payout;
        emit MaxPayoutChanged(_payout);
    }

And in deposit(), epoch is increased only when terms[epoch].payoutTotal == maxPayout.

File: MuteBond.sol
193:         if(terms[epoch].payoutTotal == maxPayout){
194:             terms.push(BondTerms(0,0,0));
195:             epochStart = block.timestamp;
196:             epoch++;
197:         }

So when the owner is going to decrease maxPayout for some reason, the below scenario would be possible by a malicious user.

1. Currently, epoch = 1, maxPayout = 1000, terms[epoch].payoutTotal = 500
2. The owner decided to decrease maxPayout to 800 for some reason and called setMaxPayout() with _payout = 800.
3. After noticing it, a malicious user frontruns deposit() with value = 400.
4. After deposit() is called, terms[epoch].payoutTotal = 900 and maxPayout will be 800.
5. Then that, deposit() will revert because maxDeposit() reverts due to uint underflow.

    function maxDeposit() public view returns (uint) {
        return maxPayout.sub(terms[epoch].payoutTotal);
    }

As a result, the contract doesn't work until the owner increases back maxPayout.

It would be more serious when it takes certain time to execute the admin functions as it should be approved by DAO.

Tools Used

Manual Review

Recommended Mitigation Steps

setMaxPayout() should check if the updated maxPayout isn't less than the current epoch's payoutTotal.

    function setMaxPayout(uint _payout) external {
        require(msg.sender == customTreasury.owner());
        require(_payout >= terms[epoch].payoutTotal); //++++++++++++++++++++

        maxPayout = _payout;
        emit MaxPayoutChanged(_payout);
    }

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

mattt21 marked the issue as sponsor confirmed

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked issue #14 as primary and marked this issue as a duplicate of 14