search

code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

Reentrancy issue. User can easily mint more than allowed presale, bypassing merkle root limit #2046

Closed captainmangoC4 closed 6 months ago

captainmangoC4 commented 6 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L220 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L192-L199

Vulnerability details

Impact

Lack of reentrancy protection and code not follow Checks, Effects, Interactions pattern guideline.

Here are the Effects stuff happen after Interactions affected by reentrancy:

Only presale tracking part have anything to do with possible finance damage, because user can bypass presale limit tracked by merkleroot.

The only one hurt by this exploit is NFT project use sale option 3 where price increase for each token mint. If user can bypass presale limit, they can mint at much lower price than other user. It would be unfair to other user who mint at the end of presale, whom might have to pay higher price than other user.

Other case include when presale price is much lower than public sale. This require admin updated price when presale end.

Proof of Concept

All NFT minting go through _mintProcessing() function https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L227-L232

It call _safeMint() which by default have callback onERC721Received(). https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/ERC721.sol#L245-L251

So anything change after _mintProcessing() can be exploited by reentrancy.

We only care about this variable tokensMintedAllowlistAddress,track token count change for presale user, after _mintProcessing() https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L192-L199

As show here this variable is used for get amount of token minted for presale user. https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L404-L406 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L212-L219

Because presale mint() function is called before tokensMintedAllowlistAddress is changed, exploiter can simply use the same merkleroot to mint again during onERC721Received() callback.

Tools Used

manual

Recommended Mitigation Steps

Add reentrancy protection

Assessed type

Reentrancy

captainmangoC4 commented 6 months ago

Issue created on behalf of judge in order to split into 2 findings

c4-judge commented 6 months ago

alex-ppg marked the issue as duplicate of #572

alex-ppg commented 6 months ago

The submission lacks sufficient quality to be graded as a rewardable duplicate of the periodic-sale re-entrancy attack vector given that it specifies only an "off-chain" impact for the said model.

c4-judge commented 6 months ago

alex-ppg marked the issue as unsatisfactory: Insufficient proof

c4-judge commented 6 months ago

alex-ppg marked the issue as unsatisfactory: Invalid