No checks for the most critical inputs when setting the collection costs could cause a huge issue to the protocol functionality

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

When setting the collection costs of each protocol, not implementing a check to evaluate that the inputs being passed to the contract, actually align with what the protocol expects in some certain way. This isn't about trusting the Collection Admin, it is about making sure the critical parameters that the protocol depends on are actually on track. The impact this kind of issue could have on the protocol could be huge as the particular collection that these values were imputed into, could experience permanent DOS for their Functions and loss of Funds as the Price That was supposed to be earned by the collection will be significantly reduced.

Proof of Concept

To explore why this issue is actually a really important one, we will explore what will happen to the protocol, if any of this, I will describe below happens

A check should be made to ensure that the end mint cost is actually lower than the starting mint cost, this condition should always remain true, if it is not let's consider what might happen,

If the starting mint cost is not greater than the end mint cost, then a problem in the internal checks in the system might surface

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

like here when the getPrice function is trying to subtract from the end mint cost, to know if more periods are left to check the price for the current period they are in. In this line of code the protocol assumes that the end mint cost will always be less than the start mint cost, if that were not to be the case. This calculation could proceed in the wrong direction.

A confirmation that the rate is actually less than the starting mint cost, as this can present issues, if the rate is actually equal to the starting mint cost, in a single period the price of the collections of tokens could drop to the mint end cost in an instant. Let’s explore how this can happen and other effects this can have on the rest of the protocol

As seen in the same code above if the rate were to be equal to the difference between the start mint cost and the end mint cost, a situation could occur where the price of the collection gets instantly downgraded to the end mint cost instead of a reduction over periods of time.

An Affirmative check, that the sales option being entered in the sales option section is a sales option that is being implemented by the protocol, if the sales option that is entered into the protocol, doesn't actually exist, the problem that could occur can be huge because the protocol will not actually know what kind of implementation to implement which could lead to a DOS of the protocol or an exploit of the protocol.

Tools Used

Manual Analysis

Recommended Mitigation Steps

These critical parameters should 100% be safety checked by the protocol, to make sure it align with what the protocol expects and are not inputs that could potentially disrupt actions in the protocol. A simple required check for these parameters will actually really suffice.

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 primary issue

[alex-ppg]

The Warden specifies that the payment configurations for a collection are insufficiently sanitized and proceeds to cite some potential misbehaviors without explicitly outlining what would happen. For example, "proceeding in the other direction" in relation to the subtraction is incorrect as the code would simply revert.

Based on the above and the failure to properly illustrate an attack vector that may arise from the misconfiguration, I consider this exhibit to have insufficient proof.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Insufficient proof

[alex-ppg]

Based on the judgment of #2033, I consider submissions #1980 and #1831 to be of QA (NC) rather than invalid and am marking them with the correct overinflated severity tag given that they would be graded C.

To note, #2033 was marked as QA before going through the QA reports and would have been marked with overinflated severity as well given that all collection misconfiguration submissions have been marked as NC due to the possibility of reconfiguration.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Overinflated severity