In `MuteBond.deposit()`, users might deposit more LPs than they expected by a malicious user

[code423n4]

Lines of code

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

Vulnerability details

Impact

Users might deposit more LPs unexpectedly if a malicious user increases an epoch by frontrunning.

Proof of Concept

deposit() has a max_buy param to purchase all remaining amounts.

    function deposit(uint value, address _depositor, bool max_buy) external returns (uint) {
        // amount of mute tokens
        uint payout = payoutFor( value );
        if(max_buy == true){
          value = maxPurchaseAmount();
          payout = maxDeposit();
        } else {
          // safety checks for custom purchase
          require( payout >= ((10**18) / 100), "Bond too small" ); // must be > 0.01 payout token ( underflow protection )
          require( payout <= maxPayout, "Bond too large"); // size protection because there is no slippage
          require( payout <= maxDeposit(), "Deposit too large"); // size protection because there is no slippage
        }
        ...
    }

But this function is frontrunable and the below situation would be possible.

1. Currently, maxPayout = 1000, epoch = 1, maxDeposit() = 100. An honest user called deposit() with max_buy = true to purchase bonds for 100 payouts.
2. After noticing this, a malicious user frontruns deposit() with max_buy = true, and the epoch was increased to 2 now. Without the malicious user, it would be possible during the normal purchases.
3. So for the honest user, epoch = 2, payout = maxDeposit() = 10000 and it will try to transfer LP tokens for 10000 payouts from the user.
4. If the honest user doesn't have enough LP balance or didn't approve that amount, the transfer will revert which is not so bad.
5. But if the honest user has enough balance and approved type(uint256).max like this test, he will be suffered to use more LPs than his expectations.

Tools Used

Manual Review

Recommended Mitigation Steps

I think deposit() should have an epoch or max_value_to_pay param to protect users when max_buy = true.

And it should revert if the epoch was increased or the final value is greater than the upper limit of value(max_value_to_pay).

[c4-judge]

Picodes marked the issue as duplicate of #25

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 3 (High Risk)