-
Notifications
You must be signed in to change notification settings - Fork 3
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
getPrice may return the wrong price #1391
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-1275
edited-by-warden
partial-50
Incomplete articulation of vulnerability; eligible for partial credit only (50%)
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
sufficient quality report
This report is of sufficient quality
Comments
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 13, 2023
c4-pre-sort
added
the
primary issue
Highest quality submission among a set of duplicates
label
Nov 15, 2023
141345 marked the issue as primary issue |
This was referenced Nov 15, 2023
Closed
Users will may end up paying more for NFTs than they should in the linear descending sale model
#499
Closed
Incorrect returned value from
getPrice()
for salesOption = 2
if block.timestamp = publicEndTime
#837
Closed
Closed
a2rocket (sponsor) confirmed |
c4-sponsor
added
the
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
label
Nov 23, 2023
Open
141345 marked the issue as sufficient quality report |
c4-pre-sort
added
the
sufficient quality report
This report is of sufficient quality
label
Nov 26, 2023
alex-ppg marked issue #1275 as primary and marked this issue as a duplicate of 1275 |
c4-judge
added
duplicate-1275
partial-50
Incomplete articulation of vulnerability; eligible for partial credit only (50%)
and removed
primary issue
Highest quality submission among a set of duplicates
labels
Dec 5, 2023
alex-ppg marked the issue as partial-50 |
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-1275
edited-by-warden
partial-50
Incomplete articulation of vulnerability; eligible for partial credit only (50%)
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
sufficient quality report
This report is of sufficient quality
Lines of code
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L540
Vulnerability details
Impact
block.timestamp = collectionPhases.allowlistStartTime
|| block.timestamp = collectionPhases.publicEndTime
getPrice
returns the wrong priceProof of Concept
When
salesOption==2
in thegetPrice
function, there is no case where the time is equal to the start/end time.When block.timestamp is equal to
allowlistStartTime
orpublicEndTime
, the default price is returned instead of the price over time.It may be correct to return the default time for
allowlistStartTime
, but it is wrong to return the default price forpublicEndTime
.In NextGenMinterContract#mint the judgment of time is:
>=
<=
So when block.timestamp = publicEndTime/allowlistStartTime, the mint parameter is set correctly.
Tools Used
vscode manual
Recommended Mitigation Steps
Assessed type
Other
The text was updated successfully, but these errors were encountered: