howlbot-integration[bot] commented 2 months ago

Lines of code

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

Vulnerability details

Impact

DOS of any user trying to stake to the same LandLord of the attacker -> DOS for that LandLord for earning taxRate from those users

Proof of Concept

The problem arises from the fact that there is no timeLock on transferToUnoccupiedPlot

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 on transferToUnoccupiedPlot 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 call transferToUnoccupiedPlot with same 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 transferToUnoccupiedPlot to the original Plot he was on

The above example can be repeated to multiple users that are willing to stake on this specific LandLord in the same Txn with the same tokenId.

Tools Used

Manual Review

Recommended Mitigation Steps

Put a time lock on transferToUnoccupiedPlot.

Assessed type

DoS

c4-judge commented 2 months ago

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