DOS for collections displaying is possible

[code423n4]

Lines of code

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

Vulnerability details

The ability to add any number of any NFT to a collection allows for the griefing attack on any collections UI.

Bob the attacker can create bogus NFT, with an enormous number of ids and gobble() all of them, making getCopiesOfArtGobbledByGobbler mapping huge. Bob's gains aren't monetary, but he can be, as an example, a rival project tries to disabling the competition, and so on.

Any front system that aims to displays the gobbled art collection becomes unavailable as long as it tries to process Bob's gobbler. Setting the severity to medium as collection displaying is a part of the system and that's an easy deterministic way to make it unavailable.

Proof of Concept

gobble() only checks that it's the owner who called and NFT isn't this GobblersERC721:

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

        // The caller must own the gobbler they're feeding.
        if (owner != msg.sender) revert OwnerMismatch(owner);

        // Gobblers have taken a vow not to eat other gobblers.
        if (nft == address(this)) revert Cannibalism();

        unchecked {
            // Increment the # of copies gobbled by the gobbler. Unchecked is
            // safe, as an NFT can't have more than type(uint256).max copies.
            ++getCopiesOfArtGobbledByGobbler[gobblerId][nft][id];
        }

This way any number of any NFTs can be added to the getCopiesOfArtGobbledByGobbler, making it arbitrary large up to DOS enabling numbers.

Recommended Mitigation Steps

Consider introducing a whitelist on the level of any art gallery displaying functionality.

Possibly introduce the disclaimer to the docs so any third party displaying UI be aware of the additional caution required here.

[Shungy]

Contract is fine here. And obviously the UI would be whitelist based. This is not a bug. This may at most be a reminder to include in a QA report.

[GalloDaSballo]

I think this can be made into a valid report after the project launches, however we have no information about the UI and it's functioning is not in-scope.

A reasonably skilled UI dev, knows that everything can be paged (paged requests to the graph for example), meaning that while infinite tokens can be added, they can be processed reasonably.

In either case, the system in scope cannot be Dossed, for this reason I'm closing as invalid