deposit() might fail to enforce the minimum ``payout`` constraint near the end of an epoch.

[code423n4]

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. deposit() might fail to enforce the minimum payout constraint near the end of an epoch.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

The deposit() function has the constraint that payout must be > 0.01 payout token (underflow protection); see L161:

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

The problem of the function is that it assumes such minimum constraint will always be met for the case of max_buy = true. This is not true.

Near the end of an Epoch, maxDeposit() might become very small to the point that maxDeposit() < ((10**18) / 100). If that happens, the deposit() function might deposit with a payOut < (10**18) / 100), which means, the minimum deposit constraint is violated.

Tools Used

VSCode

Recommended Mitigation Steps

Move the minimum constraint check outside to cover both cases; we also remove the check ``payout <= maxPayout" since it implied by the next check.

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
        }

+       require( payout >= ((10**18) / 100), "Bond too small" ); // must be > 0.01 payout token ( u+

[c4-sponsor]

mattt21 marked the issue as sponsor disputed

[mattt21]

We do not check for minimum payout with max buy set to true because max buy is meant to enforce the bond to move into the next epoch regardless of how much the payout is. If we moved that require statement, it would break the bond contract and prevent future epochs. Our UI handles max buy events when payouts are too small or the user wants the max bond size.

[chaduke3730]

got it, understandable

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

Downgrading to QA as the finding is interesting and technically valid. However, it seems to be by design and the mitigation can't be implemented without the risk of breaking the contract.