Platform: Code4rena
Start Date: 06/01/2022
Pot Size: $60,000 USDC
Total HM: 20
Participants: 33
Period: 7 days
Judge: LSDan
Total Solo HM: 9
Id: 67
League: ETH
Rank: 14/33
Findings: 2
Award: $1,413.46
π Selected for report: 2
π Solo Findings: 0
π Selected for report: jayjonah8
Also found by: bugwriter001, camden, palina, sirhashalot
232.4551 USDC - $232.46
sirhashalot
The Claimers.sol contract calls the _mint()
function from the OpenZeppelin ERC721.sol file. The OpenZeppelin comments for this function state "WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible". The _safeMint()
function helps to confirm that a contract successfully receives a ERC721 token using the receiver's OnERC721Received()
function, and will revert if the ERC721 token is not received as expected.
In contrast, the Depositors.sol function properly uses _safeMint()
instead of _mint()
.
The call to _mint()
is at line 63 of the Claimers.sol file
Replace the _mint()
call with a call with _safeMint()
to make use of the _checkOnERC721Received()
function in OpenZeppelin's _safeMint()
implementation
#0 - r2moon
2022-01-11T15:59:41Z
π Selected for report: sirhashalot
590.4972 USDC - $590.50
sirhashalot
The comment above the nextId state variable describes it as "ID of the next NFT to mint". This is not correct because the nextId state variable is never used. Based on code4rena severity, incorrect comments are considered low risk.
The issue is at line 26 of the Depositors. file
The fix depends on whether nextId is still desired to hold the "ID of the next NFT to mint".
#0 - r2moon
2022-01-11T15:58:48Z
#1 - dmvt
2022-01-29T12:36:47Z
The two issues reported are different. #45 reports the unused variable. This reports the comment.
#2 - naps62
2022-02-16T14:15:50Z
This has since been refactored away
π Selected for report: sirhashalot
590.4972 USDC - $590.50
sirhashalot
The _beforeTokenTransfer()
function in Claimers.sol contains an inaccurate require string stating Claimers: cannot burn this NFT
. This message is not factually correct becase an NFT holder can use the _burn()
function from the OpenZeppelin ERC721 library to burn this NFT if they so choose. This issue was considered equivalent to an incorrect comment, which is why this is rated as low risk.
The issue is at line 106 of the Claimers.sol file
Change the require message to something like Claimers: cannot transfer to address(0)
#0 - naps62
2022-01-11T18:31:13Z
The _burn
function is internal, and not exposed on this contract. If nfts were burnable, it would be a much more severe issue for our use case