Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Invalid

Function send() in fulfillRandomWords() may fail even for EOA winners when deployed on zkSync, thus permanently bricking the protocol

Summary

send() behaves differently on zkSync than Ethereum main-net due to differing gas price logic. This can lead to reverts for typical zkSync account types. A revert of fulfillRandomWords() will permanently brick the protocol.

Vulnerability Details

The contract is supposed to be deployed to the zkSync roll-up. fulfillRandomWords() uses vyper's send() to transfer the prize earnings to the winner. send() does not include any gas stipend by default ( https://docs.vyperlang.org/en/v0.3.10/built-in-functions.html ). This can lead to reverts on zkSync for different account types including EOAs due to dynamic gas pricing ( https://docs.zksync.io/build/quick-start/best-practices.html ).

Once fulfillRandomWords() fails for the first time, 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

Medium: If the contract is not expected to be functional when deployed to a target blockchain other than Ethereum main-net, this has been considered as a "Medium" vulnerability before (for example, https://www.codehawks.com/finding/clqqv2syu00204d0wpgq5oza7 ).

Tools Used

Manual code inspection.

Recommendations

zkSync's docs recommend replacing send() with raw_call() and forward all available gas. However, that would still allow the function call to revert leading to permanent lock of all funds of the current raffle round.

Instead, consider implementing a withdrawPrize() function that is separate from fulfillRandomWords() and sends funds after a winner is drawn and only if msg.sender==self.recent_winner so that winners have to explicitly withdraw their earned prize in a separate transaction. This removes the risk of fulfillRandomWords() reverting.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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