First Flight #11: Snek-Raffle

Summary

Calling the burn() function on any existing NFT will decrease the total supply variable which is used to mint new NFTs. This can lead to an attempt of minting an already existing ID which will make fulfillRandomWords() revert and brick the protocol as it is stuck in the RaffleState.CALCULATING state.

Vulnerability Details

Calling the burn() function on any existing NFT will decrease the total supply variable as returned by the internal _total_supply() function. This function is used to determine the new token ID in fulfillRandomWords():

ERC721._mint(recent_winner, ERC721._total_supply())

Assume, the current total supply is 3, then there are three NFTs with token IDs 0, 1, and 2. If the owner of token 0 calls the burn() function, the total supply will be reduced to 2. When the next raffle concludes, the VRF coordinator will attempt to call fulfillRandomWords() which calls ERC721._mint() which will revert when attempting to mint an already existing token ID.

The VRF coordinator will not send another transaction after a failed transaction. The Chainlink VRF v2 documentation states: "fulfillRandomWords must not revert." ( https://docs.chain.link/vrf/v2/security#fulfillrandomwords-must-not-revert , last accessed on 3/14/2024).

This leaves the protocol permanently bricked as it is stuck in the RaffleState.CALCULATING state.

Impact

High: The burn() function can be called by any NFT owner or approved account. As long as the burned NFT is not the most recently minted one, this will advertently or inadvertently lock the raffle mechanism. The deposited funds from the current raffle round will be stuck in the contract forever.

Tools Used

Manual code inspection. VRF documentation ( https://docs.chain.link/vrf/v2/security#fulfillrandomwords-must-not-revert , last accessed on 3/14/2024).

Recommendations

Consider whether exposing a public burn() function is necessary. If not, it can be removed. Otherwise, consider keeping track of the number of minted tokens separately from the _total_supply() variable to determine the ID of newly minted tokens. This ensures that new IDs are ever-increasing indepdent of burns.