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

Raffle will never conclude its current round if the winner is a smart contract without payable `__default__` function

shikhar229169

Summary

snek_raffle::fulfillRandomWords is implemented to send the rewards to the winner. But if the winner address is a smart contract without any payable __default__ function will make the call to revert making the chainlink request to go in vain, making the contract gets stuck as the raffle_state was set to calculating previously while requesting random words. Resulting in stuck contract and stuck eth of all participants without any recovery possible.

Note: If the fulfillRandomWords() implementation reverts, the VRF service will not attempt to call it a second time.

Vulnerability Details

  • The vulnerability is present in the fulfillRandomWords function where it has the implementation to send the rewards to the winners.

  • A winner being a smart contract without payable __default__ function will make the call to revert.

  • fulfillRandomWords is called by chainlink VRF coordinator and this function should never revert because if it reverts it won't be called again by chainlink VRF and a new request has to be placed.

  • But a new request can never be placed as the raffle_state was set to CALCULATING and no further request can be made to VRF coordinator.

  • Along with that rawFulfillRandomWords is expected to be only callable by chainlink's VRF coordinator, therefore can't be called manually.

  • Thus, leading the snek_raffle contract into a totally stuck state from which no recovery is possible, and all the entrance fees of participants will be stuck forever as no winner selection request can be placed.

Impact

snek_raffle contract will be in a stuck state with all the participant's entrance fees stuck in it and no winner selection request can be placed.

Tools Used

Manual Review

Recommendations

Prefer using pull over push method.
Modify the contracts as below:

diff --git a/contracts/snek_raffle.vy b/contracts/snek_raffle.vy
index 34366c4..cda3b44 100644
--- a/contracts/snek_raffle.vy
+++ b/contracts/snek_raffle.vy
@@ -76,6 +76,8 @@ last_timestamp: uint256
recent_winner: address
players: DynArray[address, MAX_NUMBER_OF_PLAYERS]
raffle_state: RaffleState
+totalEntranceFeesReceived: uint256
+winningsToWithdraw: HashMap[address, uint256]
# Events
event RequestedRaffleWinner:
@@ -115,6 +117,7 @@ def enter_raffle():
assert msg.value == ENTRANCE_FEE, ERROR_SEND_MORE_TO_ENTER_RAFFLE
assert self.raffle_state == RaffleState.OPEN, ERROR_RAFFLE_NOT_OPEN
self.players.append(msg.sender)
+ self.totalEntranceFeesReceived += ENTRANCE_FEE
log RaffleEntered(msg.sender)
@external
@@ -123,7 +126,7 @@ def request_raffle_winner() -> uint256:
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
+ has_balance: bool = self.totalEntranceFeesReceived > 0
assert is_open and time_passed and has_players and has_balance, ERROR_NOT_ENDED
self.raffle_state = RaffleState.CALCULATING
@@ -155,7 +158,14 @@ def fulfillRandomWords(request_id: uint256, random_words: uint256[MAX_ARRAY_SIZE
self.tokenIdToRarity[ERC721._total_supply()] = rarity
log WinnerPicked(recent_winner)
ERC721._mint(recent_winner, ERC721._total_supply())
- send(recent_winner, self.balance)
+ self.winningsToWithdraw[recent_winner] += self.totalEntranceFeesReceived
+ self.totalEntranceFeesReceived = 0
+
+@external
+def withdraw_winnings():
+ amountToWithdraw: uint256 = self.winningsToWithdraw[msg.sender]
+ self.winningsToWithdraw[msg.sender] = 0
+ send(msg.sender, amountToWithdraw)
#####################
# View Functions #
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.