Missing `onlyCreator` modifier on `mint`ing functions

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/NFTCollection.sol#L129-L131 https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/NFTCollection.sol#L123-L223

Vulnerability details

Impact

Missing onlyCreator modifier or any access-control to the NFTCollection.sol mint function makes all of the contract's minting functions absolutely open to any user.

Proof of Concept

All minting functions in the NFTCollection contract:

• mintAndApprove
• mintWithCreatorPaymentAddress
• mintWithCreatorPaymentAddressAndApprove
• mintWithCreatorPaymentFactory
• mintWithCreatorPaymentFactoryAndApprove

rely on the main mint function. The omission of the onlyCreator modifier one line 129 is a prerequisite for minting unlimited amount of tokens that will make all the whole collection useless.

Tools Used

VSCode

Recommended Mitigation Steps

Add the onlyCreator modifier to the mint function on line 129 in the NFTCollection.sol contract. It alone must be enough to protect all the other minting functions as well.

[HardlyDifficult]

Invalid. Each of the functions listed call into the _mint helper (directly or after another hop). That helper includes the onlyCreator modifier.

We choose to only have the modifier included once on the internal _mint function itself in order to simplify reviews and testing. I understand how it could be overlooked if the reader is not tracing the call steps all the way through -- to help avoid confusion, we do have a comment included "@dev This is only callable by the collection creator/owner."

[HickupHH3]

Agree with sponsor, modifier is in the sub-function. Great that the inline comment has been added to clear confusion.