mintLegendaryGobbler() can burn unreveal Gobbler, lead to loss emissionMultiple

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L435

Vulnerability details

Impact

when Gobbler unreveal , Gobbler.emissionMultiple==0,mintLegendaryGobbler() does not prevent burn unrevealed Gobbler, which will result in the loss emissionMultiple

Proof of Concept

    function mintLegendaryGobbler(uint256[] calldata gobblerIds) external returns (uint256 gobblerId) {
    ..

           
            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;
                 //******** No check id is not revealed,  getGobblerData[id].emissionMultiple == 0 ********//

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

    ..                

Tools Used

Recommended Mitigation Steps

    function mintLegendaryGobbler(uint256[] calldata gobblerIds) external returns (uint256 gobblerId) {
    ..

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

                if (id >= FIRST_LEGENDARY_GOBBLER_ID) revert CannotBurnLegendary(id);
+++             if (id > gobblerRevealsData.lastRevealedId) revert CannotBurnUnreveal(id);

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

                burnedMultipleTotal += getGobblerData[id].emissionMultiple;

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

    ..                

[zobront]

This is accurate, but is only a way for users to harm themselves. I'd call it QA, or Medium severity at best.

[Shungy]

Duplicate: #363

[Shungy]

This is accurate, but is only a way for users to harm themselves. I'd call it QA, or Medium severity at best.

I sort of agree. Because people might still want to use unrevealed ones if they are on a pinch to get legendary. As a reveal might need to wait up to a day. All the recommendations just say "do not allow minting with unrevealed", and some incorrectly say "just reveal during the legendary mint" (this would revert most of the time). My recommendation would be to have an extra bool argument in mint legendary function as a flag for allowing or disallowing using unrevealed ones. This could allow people to use unrevealed ones if they go through an "advanced settings" modal in the UI and enable it that way if they accept the consequences.

[GalloDaSballo]

#254

[GalloDaSballo]

L