Transferring a tokenId to an unoccupied plot doesn't make its dirty state set to false

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-munchables/blob/main/src/managers/LandManager.sol#L213

Vulnerability details

Description

The transferToUnoccupiedPlot function allows users that already have staked token ids to a landlord's land to transfer to a different plot on it. This function, however, lacks toilerState dirty updates, as it ensures the target plotId is available for the user to stake, but doesn't ensure the token ids' toiler state isn't dirty after the end of function:

function transferToUnoccupiedPlot(
...
if (plotId >= totalPlotsAvail) revert PlotTooHighError();
...
}

Impact

Users with dirty toiler states may transfer to an unoccupied plot but still won't be able to earn rewards on their farm actions.

Proof of Concept

Consider Bob has staked a token at PlotId #5 of a random landowner. The landowner unlocks some of his assets and now Bob's toilerState will become dirty as soon as he does an operation at the LandManager contract. Attempting to keep his stake going, Bob calls transferToUnoccupiedPlot. However, as the forceFarmPlots modifier is ran, his toiler's state dirty variable is set to true. Then at the transferToUnnocupiedPlot function is executed, but since it doesn't update the dirty state to false, Bob is not able to farm on a plotId that is supposed to be farmable.

Tools Used

Manual review

Recommended Mitigation Steps

Make sure the transferToUnnocupiedPlot function makes the toilerState dirty variable for the tokenId set to false, as the function already does sanity checks to ensure the plotId should be farmed.

Assessed type

Error

[c4-judge]

alex-ppg changed the severity to 3 (High Risk)

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as partial-75