It's possible to mint more NFTs than _maxAllowance due to lack of re-entrancy protection

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L196
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L189
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L193-L195

Vulnerability details

Impact

Functions MinterContract.mint() and NextGenCore.mint() lack re-entrancy protection. Moreover, NextGenCore.mint() violates the CEI pattern. As a result it's possible to mint more NFTs than permitted by the _maxAllowance parameter for MinterContract.mint().

A user who is eligible for the NFT mint during phase 1 can call MinterContract.mint() and present Merkle proof along with other parameters, including _maxAllowance. The leaf should be in the Merkle tree and incorporate msg.sender, _maxAllowance, and _tokenData.

Furthermore, the user shouldn't mint more than _maxAllowance and gencore.retrieveTokensMintedALPerAddress(col, msg.sendert) holds the number of NFTs minted so far.

Eventually, gencore.mint() is called.

Yet in gencore.mint() the value of tokensMintedAllowlistAddress is updated only after mintProcessing() which internally invokes _safeMint() with a callback. Therefore, a malicious minter could exploit callback from _safeMint() to call MinterContract.mint() recursively with the same Merkle proof to mint any number of NFTs, bypassing _maxAllowance upper bound.

Proof of Concept

• https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L196
• https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L189
• https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L193-L195
• https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L314

Tools Used

Manual review.

Recommended Mitigation Steps

Implement re-entrancy protection for MinterContract.mint() and NextGenCore.mint().

Assessed type

Reentrancy

[c4-pre-sort]

141345 marked the issue as duplicate of #51

[c4-pre-sort]

141345 marked the issue as duplicate of #1742

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as partial-50