Malicious user/users may frontrun new staker with transferToUnoccupiedPlot() in order to DOS the stakeMunchables() function

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-munchables/blob/94cf468aaabf526b7a8319f7eba34014ccebe7b9/src/managers/LandManager.sol#L199-L202

Vulnerability details

Impact

Malicious user/users may frontrun new staker with transferToUnoccupiedPlot() in order to DOS the stakeMunchables() function

Proof of Concept

If we have Alice as future staker Bob as current staker that griefs Alice:

1. Bob has staked a munchable.
2. Bob is observing the mempool very carefully
3. Alice calls stakeMunchables(plotId = 10)
4. Bob sees that and frontrun Alice with transferToUnoccupiedPlot(plotId = 10) costing him litteraly nothing as he accrues new rewards on the way.
5. Alice reverts as plot is occupied
6. Alice calls stakeMunchables(plotId = 11)
7. Bob sees that and frontrun Alice with transferToUnoccupiedPlot(plotId = 10)
8. And so on

Impact would be bigger if more malicious users are involved.

This grief could continue for large amounts of time without serious losses for Bob while Alice is denied to play the game. The incentive for the griefer is that a landlord could have great tax rates for the stakers and griefer would like to stake multiple munchables "reserving" the plots.

Likelihood = medium (grief) Impact = high (dos) Severity = high

Tools Used

Manual review

Recommended Mitigation Steps

Add wait period between transfers.

Assessed type

DoS

[0xinsanity]

fixed, but did not agree with the solution. i would prefer to only allow transfer only allowed for dirty: https://github.com/munchablesorg/munchables-common-core-latest/pull/53

[alex-ppg]

The Warden and its duplicates have outlined potential Denial-of-Service attacks that arise from the plotId being user-defined, permitting pending transactions to be hijacked by staking on the plotId they intended to before them.

These types of attacks fall under the "waste gas to waste gas" concept and thus cannot be considered any severity beyond QA as the attacker would need to pay for their meaningless transaction to cause the victim's transaction to fail and would have to do so repeatedly.

If only a single plot is available, a "gas war" that permits the first to execute their transaction to acquire the plot is considered desirable behavior.

[c4-judge]

alex-ppg changed the severity to QA (Quality Assurance)

[c4-judge]

alex-ppg marked the issue as grade-c

[jes16jupyter]

Hi @alex-ppg . Thanks for judging. I would like to respectfully disagree with the severity assessment.

The core issue here is not merely waste gas to waste gas but rather waste gas to DOS (Denial of Service). Since the stake operation of a certain user could be permanently targeted (as there is no minimum staking time) by a malicious user, the user is unable to perform any staking operations. This leads to a significant downgrade in functionality for those users. This issue cannot be resolved by the user simply resending the transaction.

Based on the above points, I would suggest that this issue should be reconsidered as a valid medium-severity vulnerability.

[alex-ppg]

Hey @jes16jupyter, thank you for your PJQA contribution. This type of issue falls precisely under the ones extensively discussed as C4 organization issues and relates to waste of gas for waste of gas.

The Denial of Service you describe is impermanent and atomic (i.e. we perform a single transaction to deny service to a single transaction). This approach is unscalable, as the would-be attacker would continuously expend funds without a real reward.