I've competed in this contest between 18/10/22-25/10/22 and achieved first place. Forgeries is an NFT raffling platform. Repo is here.
HIGH: 2
MED: 1
HIGH: Draw organizer can rig the draw to favor certain participants such as their own account. 🚩
Description
In RandomDraw, the host initiates a draw using startDraw() or redraw() if the redraw draw expiry has passed. Actual use of Chainlink oracle is done in _requestRoll:
request.currentChainlinkRequestId = coordinator.requestRandomWords({
keyHash: settings.keyHash,
subId: settings.subscriptionId,
minimumRequestConfirmations: minimumRequestConfirmations,
callbackGasLimit: callbackGasLimit,
numWords: wordsRequested
});Use of subscription API is explained well here. Chainlink VRFCoordinatorV2 is called with requestRandomWords() and emits a random request. After minimumRequestConfirmations blocks, an oracle VRF node replies to the coordinator with a provable random, which supplies the random to the requesting contract via fulfillRandomWords() call. It is important to note the role of subscription ID. This ID maps to the subscription charged for the request, in LINK tokens. In our contract, the raffle host supplies their subscription ID as a parameter. Sufficient balance check of the request ID is not checked at request-time, but rather checked in Chainlink node code as well as on-chain by VRFCoordinator when the request is satisfied. In the scenario where the subscriptionID lacks funds, there will be a period of 24 hours when user can top up the account and random response will be sent:
"Each subscription must maintain a minimum balance to fund requests from consuming contracts. If your balance is below that minimum, your requests remain pending for up to 24 hours before they expire. After you add sufficient LINK to a subscription, pending requests automatically process as long as they have not expired."
The reason this is extremely interesting is because as soon as redraws are possible, the random response can no longer be treated as fair. Indeed, Draw host can wait until redraw cooldown passed (e.g. 1 hour), and only then fund the subscriptionID. At this point, Chainlink node will send a TX with the random response. If host likes the response (i.e. the draw winner), they will not interfere. If they don't like the response, they can simply frontrun the Chainlink TX with a redraw() call. A redraw will create a new random request and discard the old requestId so the previous request will never be accepted.
function fulfillRandomWords(
uint256 _requestId,
uint256[] memory _randomWords
) internal override {
// Validate request ID
// <---------------- swap currentChainlinkRequestId --->
if (_requestId != request.currentChainlinkRequestId) {
revert REQUEST_DOES_NOT_MATCH_CURRENT_ID();
}
...
}//<------ redraw swaps currentChainlinkRequestId --->
request.currentChainlinkRequestId = coordinator.requestRandomWords({
keyHash: settings.keyHash,
subId: settings.subscriptionId,
minimumRequestConfirmations: minimumRequestConfirmations,
callbackGasLimit: callbackGasLimit,
numWords: wordsRequested
});Chainlink docs warn against this usage pattern of the VRF -"Don’t accept bids/bets/inputs after you have made a randomness request". In this instance, a low subscription balance allows the host to invalidate the assumption that 1 hour redraw cooldown is enough to guarantee Chainlink answer has been received.
Impact
Draw organizer can rig the draw to favor certain participants such as their own account.
Proof of Concept
Owner offers a BAYC NFT for holders of their NFT collection X. Out of 10,000 tokenIDs, owner has 5,000 Xs. Rest belong to retail users.
owner subscriptionID is left with 0 LINK balance in coordinator
redraw time is set to 2 hours
owner calls startDraw() which will initiate a Chainlink request
owner waits for 2 hours and then tops up their subscriptionID with sufficient LINK
owner scans the mempool for fulfillRandomWords()
If the raffle winner is tokenID < 5000, it is owner's token
Let fulfill execute and pick up the reward
7. If tokenID >= 5000
Call redraw()
fulfill will revert because of requestId mismatch
8. Owner has 75% of claiming the NFT instead of 50%
Note that Forgeries draws are presumably intended as incentives for speculators to buy NFTs from specific collections. Without having a fair shot at receiving rewards from raffles, these NFTs user buys could be worthless. Another way to look at it is that the impact is theft of yield, as host can freely decrease the probability that a token will be chosen for rewards with this method.
Also, I did not categorize it as centralization risk as the counterparty is not Forgeries but rather some unknown third-party host which offers an NFT incentive program.
Tools Used
Manual audit Chainlink docs Chainlink co-ordinator code
Recommended mitigation steps:
The root cause is that Chainlink response can arrive up to 24 hours from the most request is dispatched, while redraw cooldown can be 1 hour+. The best fix would be to enforce minimum cooldown of 24 hours.
HIGH: Draw organizer can time draws so that user's have the illusion of fair random, but draw can be cancelled.
Description
In RandomDraw, host can call startDraw() or redraw() to request a Chainlink random number, which will be used to select the winning user. They may then collect the prize NFT using winnerClaimNFT().
The issue is that in the two draw() functions, it is never checked that the current time or the time when oracle returns the random number, is less than the recoverTimelock time. When recoverTimelock is passed, owner can always claim back the NFT using lastResortTimelockOwnerClaimNFT. Therefore, host can delay calling draw or redraw until moments before recoverTimelock. If they like the selected winner, they let them pick up the item. If they don't, they can immediately send a TX to pick up the NFT using lastResortTimelockOwnerClaimNFT. This creates an unfair illusion of fair draw which users cannot be aware of. It harms the integrity of the protocol.
Impact
Draw organizer can time draws so that user's have the illusion of fair random, but draw can be cancelled.
Recommended mitigation steps
In both draw() functions, make sure the remaining time until recoveryTimelock is enough for Chainlink answer + sensible user NFT pickup time.
MED: Protocol safeguards for time durations are skewed by a factor of 7. Protocol may potentially lock NFT for period of 7 years
Description
In VRFNFTRandomDraw.sol initialize(), the MONTH_IN_SECONDS variable is used to validate two values:
configured time between redraws is under 1 month
recoverTimelock (when NFT can be returned to owner) is less than 1 year in the future
if (_settings.drawBufferTime > MONTH_IN_SECONDS) {
revert REDRAW_TIMELOCK_NEEDS_TO_BE_LESS_THAN_A_MONTH();
}
...
if (
_settings.recoverTimelock >
block.timestamp + (MONTH_IN_SECONDS * 12)
) {
revert RECOVER_TIMELOCK_NEEDS_TO_BE_LESS_THAN_A_YEAR();
}The issue is that MONTH_IN_SECONDS is calculated incorrectly:
/// @dev 60 seconds in a min, 60 mins in an hour
uint256 immutable HOUR_IN_SECONDS = 60 * 60;
/// @dev 24 hours in a day 7 days in a week
uint256 immutable WEEK_IN_SECONDS = (3600 * 24 * 7);
// @dev about 30 days in a month
uint256 immutable MONTH_IN_SECONDS = (3600 * 24 * 7) * 30;MONTH_IN_SECONDS multiplies by 7 incorrectly, as it was copied from WEEK_IN_SECONDS. Therefore, actual seconds calculated is equivalent of 7 months.
Therefore, recoverTimelock can be up to a non-sensible value of 7 years, and re-draws every up to 7 months.
Impact
Protocol safeguards for time durations are skewed by a factor of 7. Protocol may potentially lock NFT for period of 7 years.
Recommended mitigation steps:
Fix MONTH_IN_SECONDS calculation:
uint256 immutable MONTH_IN_SECONDS = (3600 * 24) * 30;