NFT token mint can run out of gas when creating the auction if the nft founder founderPct numbers are very large. #67
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
This issue or pull request already exists
Lines of code
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L206
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L113
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L157
Vulnerability details
Impact
Detailed description of the impact of this finding.
when creating the auction, the auction contract first mint nft and hold the NFT then when the auction is settled, nft is transfered out to the user.
when token.mint() is called, the nft is supposed to be minted.
but when token.mint is called.
note this while loop,
inside _isForFounder(tokenId)
we check that if this nft token id belongs to the founder, we mint to founders,
We assign nft token id to founder in the _addFounder function
the FounderPct, can be 100 at most.
here is the key,
if token id from 0 - 99 all belongs to founder,
then if a user create auction and call mint, he has to pay the gas to mint 100 nft for the founders.
minting nft for 100 times is either very expensive, gas consuming or the transaction is running out of gas.
Proof of Concept
Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
Note in the test
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/test/utils/NounsBuilderTest.sol#L84
the founder1 owns 10 nft out of 100, founder2 owns 5 nft out of 100.
We can modify the test as our POC.
we change 10 and 5 to 50 and 50
in Token.t.sol,
https://github.com/code-423n4/2022-09-nouns-builder/blob/main/test/Token.t.sol
add the import
and the test
we run
the result is
this is expect.
we can run
to track the event emissioned.
we see
this shows that nft id 0 - 99 and belongs to founders and need to be minted before the nft is minted for auction.
this means, that user creation auction and call mint, he has to mint 100 nft token for founders.
Tools Used
Foundry
Manual Review
Recommended Mitigation Steps
we can set limit on founderPct
or when adding founders,
instead of % 100, we recommend using a large number, such as % 1000.
Or we can build a separate smart contract for founder nft distriubtion.
we can say,
for every nft id that is % 10 = 0,
mint nft to a founder nft distribution contract,
then nft founder can claim the nft.
The text was updated successfully, but these errors were encountered: