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

5 stars 3 forks source link

The getPrice function returns an incorrect price #1963

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L540

Vulnerability details

Bug Description

When a user calls one of the following functions: mint, burnToMint, or burnOrSwapExternalToMint, the current price for the NFT is calculated, depending on the collection type. The issue arises in sale model 2, specifically when a user mints an NFT in the last second, resulting in the acquisition of the NFT at an incorrect amount.

All the above functions have a time check implemented as follows:

block.timestamp >= collectionPhases[col].publicStartTime &&
            block.timestamp <= collectionPhases[col].publicEndTime

The getPrice function for sale model 2 checks as follows:

block.timestamp >
            collectionPhases[_collectionId].allowlistStartTime &&
            block.timestamp < collectionPhases[_collectionId].publicEndTime

As a result, when purchasing an NFT in the last second before the publicEndTime using sale model 2, the user is charged an incorrect price, and the transaction fails.

Impact

If the collection has a sales option of 2, the user must buy the NFT at the wrong price.

Tools Used

Manual

Recommended Mitigation Steps

Consider changing getPrice as follows:

else if (
            collectionPhases[_collectionId].salesOption == 2 &&
-            block.timestamp > collectionPhases[_collectionId].allowlistStartTime &&
-            block.timestamp < collectionPhases[_collectionId].publicEndTime

+            block.timestamp >= collectionPhases[_collectionId].allowlistStartTime &&
+            block.timestamp <= collectionPhases[_collectionId].publicEndTime
        )

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

141345 marked the issue as duplicate of #1391

c4-judge commented 10 months ago

alex-ppg marked the issue as partial-50