howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-munchables/blob/94cf468aaabf526b7a8319f7eba34014ccebe7b9/src/managers/LandManager.sol#L131-L171 https://github.com/code-423n4/2024-07-munchables/blob/94cf468aaabf526b7a8319f7eba34014ccebe7b9/src/managers/LandManager.sol#L173-L197

Vulnerability details

Impact

The attack Impact can be As far as the attacker wants. can be DOS of all users of the whole protocol, specific users or specific DOS to prevent any one to stake on specific LandLord

The incentive of the attack from the prospective of the attacker

If he DOS the whole protocol -> malicious intent only
If he DOS specific LandLord -> prevents LandLord from earning through TaxRate -> loss of points to LandLord -> users will stake to the attacker instead of the reverting LandLords
Dos to specific Player -> malicious Intent

Proof of Concept

The problem arises from the fact that there is no timeLock between stakeMunchable and unstakeMunchable

This open up the ability to malicious actor to frontrun any txn to make it revert, Here is why.

in stakeMunchable

File: LandManager.sol
131:     function stakeMunchable(
132:         address landlord,
133:         uint256 tokenId,
134:         uint256 plotId
135:     ) external override forceFarmPlots(msg.sender) notPaused 
//////////Ommit
138:         if (plotOccupied[landlord][plotId].occupied)

We check if that plotId is occupied or not. Now this check is good and works as intended BUT

Since there is no timeLock between stakeMunchable and unstakeMunchable This check can be weaponized

Example:

Alice comes and see PlotId of specific landlord is empty
Alice submits its txn to stakeMunchable
BOB (malicious attacker) sees in the memePool a user want to stake
BOB (unstake (if he is staked to a genuine other LanLord)) front run Alice and call stakeMunchable with same landlord and plotId to mark that Plot to be occupied
Alice txn reverts due to if (plotOccupied[landlord][plotId].occupied) revert OccupiedPlotError(landlord, plotId);
BOB backrun the reverted txn of Alice to unstake

The above example can be repeated to multiple users in the same Txn with the same tokenId which makes it more castrophic

Tools Used

Manual Review

Recommended Mitigation Steps

Put a time lock between stakeMunchable and unstakeMunchable

Assessed type

DoS

c4-judge commented 2 months ago

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