Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MinterContract.getPrice :: Different pricing for same collection due to improper timestamp validation #1407

Closed
c4-submissions opened this issue Nov 13, 2023 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-1275 edited-by-warden partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@c4-submissions
Copy link
Contributor

c4-submissions commented Nov 13, 2023

Lines of code

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

Vulnerability details

Impact

Severity : High (ex: a salesOption that increases price over time, will incur a heavy loss if token id minted at exact publicEndTime)

Proof of Concept

  • In MinterContract.mint function, when minting, MinterContract.getPrice can be abused for different pricing for same collection. Due to following improper timestamp validation.
  1. User mints exactly at collection's allowlistStartTime or publicEndTime.
  2. Now the price of that tokens will be different from rest of collection, because the else if block validated for current timestamp to be < or > to allowlistStartTime & publicEndTime instead of <= and >=.
  3. so if this is not fixed, the sers can mint exactly at allowlistStartTime or publicEndTime and get away with exact collectionMintCost instead of price increase or decrease due to the time difference, number of mints, rate and other collection costs parameters

Tools Used

Manual review & foundry testing

Recommended Mitigation Steps

function getPrice(uint256 _collectionId) public view returns (uint256) {
        uint tDiff;
        if (collectionPhases[_collectionId].salesOption == 3) {
            // increase minting price by mintcost / collectionPhases[_collectionId].rate every mint (1mint/period)
            // to get the price rate needs to be set
            if (collectionPhases[_collectionId].rate > 0) {
                return collectionPhases[_collectionId].collectionMintCost + ((collectionPhases[_collectionId].collectionMintCost / collectionPhases[_collectionId].rate) * gencore.viewCirSupply(_collectionId));
            } else {
                return collectionPhases[_collectionId].collectionMintCost;
            }
-       } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
+       } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp >= collectionPhases[_collectionId].allowlistStartTime && block.timestamp <= collectionPhases[_collectionId].publicEndTime){
            tDiff = (block.timestamp - collectionPhases[_collectionId].allowlistStartTime) / collectionPhases[_collectionId].timePeriod;
            uint256 price;
            uint256 decreaserate;
            if (collectionPhases[_collectionId].rate == 0) {
                price = collectionPhases[_collectionId].collectionMintCost / (tDiff + 1);
                decreaserate = ((price - (collectionPhases[_collectionId].collectionMintCost / (tDiff + 2))) / collectionPhases[_collectionId].timePeriod) * ((block.timestamp - (tDiff * collectionPhases[_collectionId].timePeriod) - collectionPhases[_collectionId].allowlistStartTime));
            } else {
                if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
                    price = collectionPhases[_collectionId].collectionMintCost - (tDiff * collectionPhases[_collectionId].rate);
                } else {
                    price = collectionPhases[_collectionId].collectionEndMintCost;
                }
            }
            if (price - decreaserate > collectionPhases[_collectionId].collectionEndMintCost) {
                return price - decreaserate; 
            } else {
                return collectionPhases[_collectionId].collectionEndMintCost;
            }
        } else {
            // fixed price
            return collectionPhases[_collectionId].collectionMintCost;
        }
    }

Assessed type

Invalid Validation

@c4-submissions c4-submissions added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 13, 2023
c4-submissions added a commit that referenced this issue Nov 13, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #1391

@c4-judge c4-judge added duplicate-1275 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) and removed duplicate-1391 labels Dec 5, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-50

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Dec 9, 2023
@c4-judge
Copy link

c4-judge commented Dec 9, 2023

alex-ppg changed the severity to 2 (Med Risk)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-1275 edited-by-warden partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Development

No branches or pull requests

4 participants