Open howlbot-integration[bot] opened 2 months ago
@koolexcrypto
The above report covers two different scenarios. The first issue is indeed covered in the issue #210 of which this is a duplicate.
The second issue is explained in the 2nd Bug Scenario:
part of the report
Wrong positioning of _incrementGeneration() will lead to the user paying the starting price of 1st nft of generation 1 even when the nft being minted is the 1st nft of a different generation.
I clubbed both these issue since they had the same origin point (minting of first nft of a different generation).
A request to consider this report to be 'selected for report' for covering both possibilities of the vulnerability & detailed explanation.
koolexcrypto marked the issue as not a duplicate
koolexcrypto marked the issue as primary issue
koolexcrypto marked the issue as selected for report
Lines of code
https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L280-L283 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L227-L232
Vulnerability details
The report covers two bugs:
_incrementGeneration()
will lead to the user paying the starting price of 1st nft ofgeneration 1
even when the nft being minted is the 1st nft of adifferent generation
.Proof of Concept
1st Bug Scenario:
generation 1
into consideration. Only direct minting can occur. No nft can be minted through forging.startPrice
(0.005 ether)._tokenIds
&generationMintCounts
is updated to1
. Meaning that one nft has been minted in the protocol.0.005 + (0.0000245 * 1)
=0.0050245 ether
._tokenIds
&generationMintCounts
is updated to2
. Meaning that as the first increment in price happens, the 2nd nft for the protocol has been minted.generation 1
when_tokenIds
&generationMintCounts
has reached9999
.10000
defined bymaxTokensPerGen
.0.005 + (0.0000245 * 9999)
=0.2499755 ether
.generationMintCounts
has not reachedmaxTokensPerGen
, the_incrementGeneration()
check is skipped._tokenIds
&generationMintCounts
is updated to10000
.Now comes the interesting part.
generation 2
because10000
nfts have already been minted at this point.generation 2
should be thestartPrice
plus thepriceIncrementByGen
that is0.005 + 0.000005
=0.005005 ether
.mintToken()
, the price he has to pay is0.005 + (0.0000245 * 10000)
=0.25 ether
(As mentioned in whitepaper).generation 2
, user has to pay the maximum price ofgeneration 1
._mintInternal()
, due to the first check, the generation is incremented fromgeneration 1
togeneration 2
._tokenIds
is updated to10001
&generationMintCounts
to1
.0.005005 ether
but instead ends up paying0.25 ether
. User's loss =0.244995 ether
.2nd Bug Scenario:
generation 2
. ThegenerationMintCounts
is10000
& no minting has taken place through forging forgeneration 2
. Meaning that nft count forgeneration 3
is zero.mintToken()
, the same situation is repeated as explained in the scenario above & the user ends up paying the max price ofgeneration 2
to mint the 1st nft ofgeneration 3
.Now if the
generationMintCounts
is at9999
&_mintNewEntity()
is called internally through forging, a different scenario takes place._tokenIds
becomes20000
&generationMintCounts
becomes10000
.if
condition & the generation is incremented.generationMintCounts
forgeneration 3
is reset to0
.mintToken()
, the price calculated will be0.005 + (0.0000345 * 0)
=0.005 ether
which the initial starting price of the 1st nft ofgeneration 1
. [priceIncrement
for gen 3 would be = 0.0000245 + 0.000005 + 0.000005 = 0.0000345]Tools Used
Manual Review
Recommended Mitigation Steps
Fixing the bugs would require changes in the
calculateMintPrice()
&_mintInternal()
. However the protocol will not be able to claim the the last calculated price as intended by the whitepaper but it would then align with the number of nfts minted & the price the user pays.Assessed type
Error