First Flight #11: Snek-Raffle

Summary

If the winner of a raffle is a smart contract which does not accept ETH or is a smart contract whose callback exceeds the gas stipend, the call to fulfillRandomWords() will revert, and the protocol will be permanently bricked.

Vulnerability Details

If the winner of raffle (recent_winner in fulfillRandomWords()) is a smart contract that does not implement a payable default function (__default()__ function in vyper with the @payable decorator), then send(recent_winner, self.balance) in fulfillRandomWords() will revert leading to a failed transaction. send() will also fail if the target is a smart contract whose callback function exceeds the gas stipend allocated by send().

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: A malicious user can attempt brick the protocol permanently for the cost of ENTRANCE_FEE. The deposited funds from the current raffle round will be stuck in the contract forever, and no further raffles can be played.

Note that for the attack to succeed the malicious user's contract must be selected as the winner. However, the user can attempt multiple raffles or enter the same raffle multiple times to increase the chances of a successful attack on the protocol.

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

Restrict participation to EOAs only by asserting that msg.sender == tx.origin. However, this comes at the expense of excluding smart wallets (like multi-sig wallets) from participating.

Alternatively, implement a withdrawPrize() function that sends funds after a winner is drawn and if msg.sender==self.recent_winner so that winners have to explicitly withdraw their earned prize. This removes the risk of fulfillRandomWords() reverting.