Art Gobblers contest - Chom's results

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L432-L442 https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L866-L868

Vulnerability details

Impact

Minting legendary gobblers not clearing burned gobblers emissionMultiple (and other properties). getGobblerEmissionMultiple also returns the old emission rate although it is burned. Contracts that use getGobblerEmissionMultiple may be impacted by duplicated emission rates.

Proof of Concept

            for (uint256 i = 0; i < cost; ++i) {
                id = gobblerIds[i];

                if (id >= FIRST_LEGENDARY_GOBBLER_ID) revert CannotBurnLegendary(id);

                require(getGobblerData[id].owner == msg.sender, "WRONG_FROM");

                burnedMultipleTotal += getGobblerData[id].emissionMultiple;

                emit Transfer(msg.sender, getGobblerData[id].owner = address(0), id);
            }

This block of code is responsible for burning gobbler. It never reset emissionMultiple and other properties of gobbler except owner.

It is still queryable here and can be used in any other part of code although it is burned

    function getGobblerEmissionMultiple(uint256 gobblerId) external view returns (uint256) {
        return getGobblerData[gobblerId].emissionMultiple;
    }

Recommended Mitigation Steps

You should delete getGobblerData[id] after it has been burned

            for (uint256 i = 0; i < cost; ++i) {
                id = gobblerIds[i];

                if (id >= FIRST_LEGENDARY_GOBBLER_ID) revert CannotBurnLegendary(id);

                require(getGobblerData[id].owner == msg.sender, "WRONG_FROM");

                burnedMultipleTotal += getGobblerData[id].emissionMultiple;

                emit Transfer(msg.sender, getGobblerData[id].owner = address(0), id);

                delete getGobblerData[id];
            }