_collectionEndMintCost should be less than collectionMintCost for the getPrice function to implement salesOption2, but there is no such check

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

_collectionEndMintCost should be less than collectionMintCost for the getPrice function to implement salesOption2, but there is no such check. Therefore getPrice suffer potential underflow during salesOption 2, for positive rate

salesOption 2 implements some depreciating auction price, either through exponential or linear decrement.
On the case of linear decrement, the formula is as follow:

tDiff = (block.timestamp - collectionPhases[_collectionId].allowlistStartTime) / collectionPhases[_collectionId].timePeriod;

...
        
        if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
                    price = collectionPhases[_collectionId].collectionMintCost - (tDiff * collectionPhases[_collectionId].rate);
                } else {
                    price = collectionPhases[_collectionId].collectionEndMintCost;
                }

which has limited the price to be collectionEndMintCost. However, when we double check the collectionEndMintCost, there is no guarantee that collectionEndMintCost is strictly less than the collectionMintCost.

    function setCollectionCosts(uint256 _collectionID, uint256 _collectionMintCost, uint256 _collectionEndMintCost, uint256 _rate, uint256 _timePeriod, uint8 _salesOption, address _delAddress) public CollectionAdminRequired(_collectionID, this.setCollectionCosts.selector) {
        require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
        collectionPhases[_collectionID].collectionMintCost = _collectionMintCost;
        collectionPhases[_collectionID].collectionEndMintCost = _collectionEndMintCost;
        collectionPhases[_collectionID].rate = _rate;
        collectionPhases[_collectionID].timePeriod = _timePeriod;
        collectionPhases[_collectionID].salesOption = _salesOption;
        collectionPhases[_collectionID].delAddress = _delAddress;
        setMintingCosts[_collectionID] = true;
    }

Impact: if the collectionEndMintCost is bigger than the collectionMintCost, and the user set salesOption2, then the auction cannot proceed since collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost revert.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps

Consider adds a validation check on collectionEndMintCost <= collectionMintCost, if collectionEndMintCost is solely used for a depreciating auction; otherwise when a salesOption 2 is wanted, validate so.

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as duplicate of #478

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1831

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Overinflated severity