Lines of code

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

Vulnerability details

Summary

A timestamp earlier than lastToilDate will cause the entire _farmPlots function to revert.

Vulnerability Details

In the _farmPlots() function, the total schnibbles for each stake are calculated unsafely within a loop using the following method:

schnibblesTotal = (timestamp - _toiler.lastToilDate) * BASE_SCHNIBBLE_RATE;

The operation (timestamp - _toiler.lastToilDate) is performed without verifying that timestamp > _toiler.lastToilDate. If _getNumPlots(landlord) is less than _toiler.plotId, the timestamp is set to the landlord's lastUpdated value:

if (_getNumPlots(landlord) < _toiler.plotId) {
    timestamp = plotMetadata[landlord].lastUpdated;
    toilerState[tokenId].dirty = true;
}

This scenario can lead to an underflow error if a user stakes on a landlord's plot where lastUpdated has not been refreshed. If the landlord's lastUpdated remains outdated and _getNumPlots(landlord) becomes less than _toiler.plotId due to price changes or intentional unlocking actions by the landlord, the user will be unable to perform any critical actions. This is because most actions call _farmPlots first. A malicious landlord could exploit this by intentionally not updating lastUpdated, effectively causing a Denial of Service (DoS) for the renter's entire operations.

Recommendation

Ensure that timestamp > _toiler.lastToilDate before performing the arithmetic operation. If this condition is not met, set schnibblesTotal to 0.

Assessed type

DoS

code-423n4 / 2024-07-munchables-findings