Art Gobblers contest - Ch_301's results

The users could check the emissionMultiple without revealing it

The user can mintFromGoo() and invoke requestRandomSeed() with toBeRevealed == 1. Now he can check emissionMultiple without invoking revealGobblers(). If he doesn't like the result of the emissionMultiple he can sell these Gobblers without revealing it

Finding

File:  /src/ArtGobblers.sol
 
 function transferFrom(
        address from,
        address to,
        uint256 id
    ) public override {

https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L880-L917

Recommended Mitigation Steps

Add check is it revealed or not before transfer() the Gobblers to a new address

Users in the allowance mapping can not mintFromGoo()

If someone has an amount on allowance list he can’t use it So he needs to transfer it first to his balance and then he can invoke mintFromGoo() or addGoo()

Finding

File: /src/ArtGobblers.sol

    function addGoo(uint256 gooAmount) external {
        // Burn goo being added to gobbler.
        goo.burnForGobblers(msg.sender, gooAmount);

        // Increase msg.sender's virtual goo balance.
        updateUserGooBalance(msg.sender, gooAmount, GooBalanceUpdateType.INCREASE);
    }

https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L770-L776 https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/Pages.sol#L195-L213 https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L368-L390

Miss to check isApprovedForAll() and getApproved()

Finding

File: /src/ArtGobblers.sol

if (owner != msg.sender) revert OwnerMismatch(owner);

https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L733 https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L437

Recommended Mitigation Steps

if(owner != msg.sender || !isApprovedForAll(owner, msg.sender) || getApproved(tokenId) != msg.sender) revert OwnerMismatch(owner);

And for mintLegendaryGobbler()

require(owner == msg.sender || isApprovedForAll(owner, msg.sender) || getApproved(Id) == msg.sender,'WRONG_FROM');

Use safeTransferFrom() rather than transferFrom()

The transaction can fail and getCopiesOfArtGobbledByGobbler will keep the value on it

Finding

File: /src/ArtGobblers.sol

: ERC721(nft).transferFrom(msg.sender, address(this), id);

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