Unbounded Tax Rate Updates Enable Griefing Attacks

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Description

The updateTaxRate function in the LandManager contract allows landlords to update their tax rate without any restrictions on frequency or magnitude of changes. This lack of limitation presents a griefing vulnerability where malicious actors can frequently alter their tax rates, potentially disrupting the game economy and frustrating legitimate players.

Vulnerability Detail

The updateTaxRate function can be called an unlimited number of times by a landlord, allowing for rapid and unrestricted changes to the tax rate.

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

function updateTaxRate(uint256 newTaxRate) external override notPaused {
    (address landlord, ) = _getMainAccountRequireRegistered(msg.sender);
    if (newTaxRate < MIN_TAX_RATE || newTaxRate > MAX_TAX_RATE)
        revert InvalidTaxRateError();
    if (plotMetadata[landlord].lastUpdated == 0)
        revert PlotMetadataNotUpdatedError();
    uint256 oldTaxRate = plotMetadata[landlord].currentTaxRate;
    plotMetadata[landlord].currentTaxRate = newTaxRate;
    emit TaxRateChanged(landlord, oldTaxRate, newTaxRate);
}

While the function includes checks for valid tax rates and initialized plot metadata, it does not implement any mechanism to prevent frequent updates or limit the magnitude of changes.

Impact

This vulnerability can be exploited to:

1. Disrupt the game economy by constantly changing tax rates, making it difficult for players to make informed decisions about land usage.
2. Frustrate legitimate players by creating an unstable and unpredictable tax environment.
3. Potentially manipulate the market by rapidly switching between extremely low and high tax rates.
4. Increase network congestion and gas costs for other players due to frequent state changes and event emissions.

Proof of Concept

A malicious landlord could write a script to repeatedly call the updateTaxRate function, alternating between MIN_TAX_RATE and MAX_TAX_RATE:

const MIN_TAX_RATE = await landManager.MIN_TAX_RATE();
const MAX_TAX_RATE = await landManager.MAX_TAX_RATE();

async function griefingAttack(attacker) {
    while (true) {
        await landManager.connect(attacker).updateTaxRate(MIN_TAX_RATE);
        await landManager.connect(attacker).updateTaxRate(MAX_TAX_RATE);
    }
}

This script, when run, would continuously flip the tax rate between its minimum and maximum values, creating chaos for any players interacting with the attacker's land.

Recommended Mitigation

1. Introduce a cooldown period between tax rate updates:

uint256 public constant TAX_UPDATE_COOLDOWN = 1 days;
mapping(address => uint256) private lastTaxUpdateTime;

function updateTaxRate(uint256 newTaxRate) external override notPaused {
    require(block.timestamp >= lastTaxUpdateTime[msg.sender] + TAX_UPDATE_COOLDOWN, "Cooldown period not elapsed");
    // ... existing code ...
    lastTaxUpdateTime[msg.sender] = block.timestamp;
}

2. Limit the magnitude of tax rate changes within a given time period:

uint256 public constant MAX_TAX_RATE_CHANGE = 10e16; // 10% change

function updateTaxRate(uint256 newTaxRate) external override notPaused {
    uint256 oldTaxRate = plotMetadata[msg.sender].currentTaxRate;
    require(abs(int256(newTaxRate) - int256(oldTaxRate)) <= MAX_TAX_RATE_CHANGE, "Tax rate change too large");
    // ... existing code ...
}

Assessed type

Invalid Validation

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Insufficient proof

[c4-judge]

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

[c4-judge]

alex-ppg marked the issue as grade-c