First Flight #2: Puppy Raffle

Summary

Reentrancy gives attacker execution flow when info for token id is updated but token is not minted, allowing him to re-mint nft with same id

Vulnerability Details

Call to the winner (bool success,) = winner.call{value: prizePool}(""); appears before the token minting _safeMint(winner, tokenId); in selectWinner. So winner in fallback function can run round again, mint NFT with the same id as uint256 tokenId = totalSupply(); is not changed, but with higher rarity that will overwrite tokenIdToRarity[tokenId]. Then burn the token and return from fallback where nft with the same id will be minted

Impact

Ability to increase rarity of nft with current id

Tools Used

Recommendations

Add nonReentrant modifier, make call to the winner only after side effects