Lines of code

Vulnerability details

Impact

The function VRFNFTRandomDraw.sol:fulfillRandomWords() called by Chainlink receives an array of random words, and uses it to choose a random offset by which the winning tokenId is selected.

The chosen tokenId is stored on the public request variable in the contract, which is publically readable by everyone. Moreover, the event DiceRollComplete is emitted with the entire request object, revealing the winning tokenId.

Since the reward token is not immediately sent to the winner, a pull mechanism is used where the winner has to call winnerClaimNFT() on the same function to claim their winnings. There is likely to be considerable delay between the dice roll being complete and the winner claiming their reward - and a redraw cannot be done within the request.drawTimelock, which appears to be a week by default.

In the scenario where the drawingToken NFT collection is highly liquid and traded often, it is easy for an attacker to listen for the DiceRollComplete event, and then purchase / flashloan the winning tokenId (currentChosenTokenId) for themselves and claim the rewardToken before the original token holder knows that they have won / has a chance to claim it for themselves.

Essentially, anyone who enters a draw will have to delist their NFTs from the market to avoid being frontrun and losing out on their winnings.

POC

VRFNFTRandomDraw.sol:fulfillRandomWords(): the winning tokenId is set to the public request variable and the event is emitted

request.currentChosenTokenId =
            (_randomWords[0] % tokenRange) +
            settings.drawingTokenStartId;

// Emit completed event.
emit DiceRollComplete(msg.sender, request);

Mitigation

Instead of storing currentChosenTokenId on the public request variable, store it separately in a private uint. Then it won’t be emitted with the event and won’t be publicly readable.

Tools used

Manual review

The text was updated successfully, but these errors were encountered:

All reactions