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

Wrong economics mechanism in the function getPrice. #643

Closed
c4-submissions opened this issue Nov 9, 2023 · 8 comments
Closed

Wrong economics mechanism in the function getPrice. #643

c4-submissions opened this issue Nov 9, 2023 · 8 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 duplicate-644 sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

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

Vulnerability details

Proof of Concept

If collectionphase is 3 and rate is greater than 0 then, the mint cost increases if collectonid’s tokenids supply/demand increase. If rate is 0 then, mintcost does not increases/decreses.

return collectionPhases[_collectionId].collectionMintCost + ((collectionPhases[_collectionId].collectionMintCost / collectionPhases[_collectionId].rate) * gencore.viewCirSupply(_collectionId));

So the problem is that minting cost incrases if collectionid’s tokenids supply increases but minting cost does not decrease if tokenid supply decreases(if the collectionid’s tokenids are burned).
This is an economically wrong mechanism because price is increased based on tokendis supply/demand but price is not decreased based on supply/demand.

Let's take an example,let assume a collectionid’s total minted tokens are 10. So minting cost will increase based on the 10 tokenids. Now 4 tokenids are burned, so minting cost should be based on the remaining 6 tokenids. but the problem is codes still calculate minting cost based on 10 tokenids.

Impact

Users will be charged more minting cost than fair prices.

Tools Used

manual review

Recommended Mitigation Steps

When multiplying totalsupply of tokenids, instead of multiplying gencore.viewCirSupply(_collectionId), multiply by gencore.totalSupplyOfCollection( _collectionID).

Assessed type

Context

@c4-submissions c4-submissions added 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 labels Nov 9, 2023
c4-submissions added a commit that referenced this issue Nov 9, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #469

@c4-pre-sort
Copy link

141345 marked the issue as not a duplicate

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Nov 16, 2023
@c4-pre-sort
Copy link

141345 marked the issue as sufficient quality report

@c4-pre-sort
Copy link

141345 marked the issue as primary issue

@c4-sponsor
Copy link

a2rocket (sponsor) disputed

@a2rocket
Copy link

this is the intended design, burnt tokens are not removed from the circulating supply and this is correct. If the total supply is 100 and the circulating supply is 100 it mints that the collection is fully funded. If someone wants to burn its token this does not mean that someone can mint it. If someone burns the token then the total supply = circulatingSupply - burntSupply. We have a function for this on the Core contract. With this logic the price should not decrease if someone burnt a token before minting ends.

@c4-judge c4-judge closed this as completed Dec 6, 2023
@c4-judge
Copy link

c4-judge commented Dec 6, 2023

alex-ppg marked the issue as duplicate of #644

@c4-judge c4-judge added duplicate-644 and removed primary issue Highest quality submission among a set of duplicates labels Dec 6, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Dec 8, 2023
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 duplicate-644 sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

5 participants