CrossMint Manual Review Findings

CMT-01M: Pre-Condition Bypass

Description:

The linked functions are meant to act as a relay between CrossMint and the deployed NFT contracts of CrossMint to ensure certain preconditions are evaluated, such as that the user has supplied a high enough mint fee or that the caller has sufficient permissions to mint the particular NFT. All these can be bypassed by the deployer of an NFT contract directly interacting with it.

Example:

Copy
157function mintCrossMint1155(
158  CrossMint1155 crossMint1155,
159  address to,
160  uint256 id,
161  uint256 amount,
162  bytes memory data
163)
164  external
165  payable
166  withPermissions(address(crossMint1155))
167  whenNotPaused
168  withMintFee
169{
170  crossMint1155.mint(to, id, amount, data);
171
172  emit Mint(to, CROSS_MINT_1155, address(crossMint1155), id, amount, "");
173}

Recommendation:

We strongly recommend the way MinterControl is utilized by the NFT contracts is revised as in its current state it allows their deployer to mint NFTs at will without paying fees or matching pre-conditions.

Alleviation:

Several steps were taken to revamp the system. Namely, the MinterControl contract now takes a single argument that is set to the msg.sender within the CrossMint721 and CrossMint1155 contracts and the factory functions and contract constructors no longer accept an owner argument. On a consequent update, the factories began validating the msg.sender to be equal to the CrossMint contract thereby prohibiting any form of circumvention from occuring.

CMT-02M: User Based Salt

Description:

The deployCrossMint721 and deployCrossMint1155 functions utilize a user-defined salt in their create2 invocation which allows the initial deployer of an NFT contract to replace it and its ContractPermission settings at any time.

Example:

Copy
245function deployCrossMint721(
246  bytes32 salt,
247  string memory name,
248  string memory symbol,
249  bool protected
250) public payable whenNotPaused withCollectionFee{
251  address crossMint721 = CrossMint721Factory.deployCrossMint721(
252    salt,
253    name,
254    symbol,
255    baseURI,
256    msg.sender
257  );
258
259  permissions[crossMint721] = ContractPermission(msg.sender, protected);
260
261  emit NFTCollectionDeployed(msg.sender, CROSS_MINT_721, crossMint721);
262}

Recommendation:

We advise the same salt to be prevented from being utilized as it can lead to misbehaviours due to create2 re-deploying on the same address.

Alleviation:

The salt is no longer user-defined and instead, a contract-level incremental ID is utilized ensuring the salt argument will be unique for each deployment invocation.