howlbot-integration[bot] commented 3 months ago

Lines of code

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

Vulnerability details

Impact

The latestTaxRate for a plot is updated after being used to calculate the landlord's schnibblesLandlord, the problem here is that if the landlord increase their tax rate, a tenant might game the system by delaying their call to the farmPlot function in other to avoid paying the new rent.


    function _farmPlots(address _sender) internal {
        (
            address mainAccount,
            MunchablesCommonLib.Player memory renterMetadata
        ) = _getMainAccountRequireRegistered(_sender);

        uint256[] memory staked = munchablesStaked[mainAccount];
        MunchablesCommonLib.NFTImmutableAttributes memory immutableAttributes;
        ToilerState memory _toiler;
        uint256 timestamp;
        address landlord;
        uint256 tokenId;
        int256 finalBonus;
        uint256 schnibblesTotal;
        uint256 schnibblesLandlord;
        for (uint8 i = 0; i < staked.length; i++) {
            timestamp = block.timestamp;
            tokenId = staked[i];
            _toiler = toilerState[tokenId];
            if (_toiler.dirty) continue;
            landlord = _toiler.landlord;
            // use last updated plot metadata time if the plot id doesn't fit
            // track a dirty bool to signify this was done once
            // the edge case where this doesnt work is if the user hasnt farmed in a while and the landlord
            // updates their plots multiple times. then the last updated time will be the last time they updated their plot details
            // instead of the first
            if (_getNumPlots(landlord) < _toiler.plotId) {
                timestamp = plotMetadata[landlord].lastUpdated;
                toilerState[tokenId].dirty = true;
            }
            (
                ,
                MunchablesCommonLib.Player memory landlordMetadata
            ) = _getMainAccountRequireRegistered(landlord);

            immutableAttributes = nftAttributesManager.getImmutableAttributes(
                tokenId
            );
            //@audit can it go out of bounds/
            //max uint256(immutableAttributes.realm) * 5 = 20, 25
            //uint256(landlordMetadata.snuggeryRealm)    =  4,  5
            finalBonus =
                int16(
                    REALM_BONUSES[
                        (uint256(immutableAttributes.realm) * 5) + uint256(landlordMetadata.snuggeryRealm)
                    ]
                ) +
                int16(
                    int8(RARITY_BONUSES[uint256(immutableAttributes.rarity)])
                );
            schnibblesTotal =
                (timestamp - _toiler.lastToilDate) *
                BASE_SCHNIBBLE_RATE;
            //@audit is this correct?
            schnibblesTotal = uint256(
                (int256(schnibblesTotal) +
                    (int256(schnibblesTotal) * finalBonus)) / 100
            )
@->            schnibblesLandlord =
                (schnibblesTotal * _toiler.latestTaxRate) /
                1e18;

            toilerState[tokenId].lastToilDate = timestamp;
@->            toilerState[tokenId].latestTaxRate = plotMetadata[_toiler.landlord]
                .currentTaxRate;

            renterMetadata.unfedSchnibbles += (schnibblesTotal -
                schnibblesLandlord);

            landlordMetadata.unfedSchnibbles += schnibblesLandlord;
            landlordMetadata.lastPetMunchable = uint32(timestamp);
            accountManager.updatePlayer(landlord, landlordMetadata);
            emit FarmedSchnibbles(
                _toiler.landlord,
                tokenId,
                _toiler.plotId,
                schnibblesTotal - schnibblesLandlord,
                schnibblesLandlord
            );
        }
        accountManager.updatePlayer(mainAccount, renterMetadata);
    }

Proof of Concept

Alice Increase her taxrate
Bob a tenant seeing this and knowing that they will have to pay more to Alice will delay farming so he can use the old tax rate which is local.

Tools Used

Manual

Recommended Mitigation Steps

Use a better mechanism to track tax.

Assessed type

Other

c4-judge commented 3 months ago

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