Art Gobblers contest - auditor0517's results

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/2ae644ace932271fb34399c1c0771aabae2e423c/src/ArtGobblers.sol#L411-L471

Vulnerability details

Impact

When a gobbler is burned for a legendary gobbler, both owner and getApproved should be deleted.

But getApproved isn't deleted now so users can revive the gobbler again.

Proof of Concept

Users can set getApproved[id] for the owned NFTs.

File: 2022-09-artgobblers\src\utils\token\GobblersERC721.sol
092:     function approve(address spender, uint256 id) external {
093:         address owner = getGobblerData[id].owner;
094: 
095:         require(msg.sender == owner || isApprovedForAll[owner][msg.sender], "NOT_AUTHORIZED");
096: 
097:         getApproved[id] = spender;
098: 
099:         emit Approval(owner, spender, id);
100:     }

After the NFT is burned for a legendary gobbler, they can revive using transferFrom() from address(0) as getApproved wasn't deleted.

After all, they can get more emissionMultiple and gobblersOwned from this part.

File: 2022-09-artgobblers\src\ArtGobblers.sol
910:             getUserData[to].lastBalance = uint128(gooBalance(to));
911:             getUserData[to].lastTimestamp = uint64(block.timestamp);
912:             getUserData[to].emissionMultiple += emissionMultiple;
913:             getUserData[to].gobblersOwned += 1;
914:         }

Tools Used

Manual Review

Recommended Mitigation Steps

Recommend changing this part.

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 getApproved[id];
}