Lottery owner can rig the draw to win the jackpot by swapping the source

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/RNSourceController.sol#L89-L104,https://github.com/code-423n4/2023-03-wenwin/blob/main/src/VRFv2RNSource.sol#L32-L38,https://github.com/code-423n4/2023-03-wenwin/blob/main/src/RNSourceBase.sol#L33-L46

Vulnerability details

Lottery owner can rig the draw to win the jackpot by swapping the source

Impact

The lottery owner has the ability to swap the Random Source under certain cirumstances, and this can be exploited to set a new source contract that returns any number set by it. This way, it can claim the jackpot prize, or any other prizes, by previously buying the corresponding tickets.

Although mentioned the [M-1] Centralization Risk for trusted owners, the risk presented here involves major asset losses for the whole system, affecting its economy, and all of its users.

Wenwin puts a heavy enfasis on transparency for its users, making sure that no "rug pull" can happen on the initial sale,
going for the immutability of their contracts, while trying to build decentralized and trustless games with infinite scalability.

A similar issue evaluated on a previous Code4rena contest can be found here.

Proof of Concept

From the Wenwin documentation:

"It's important to understand that the deployer cannot change the randomness source other than in the unlikely scenario in which the Chainlink oracle fails repeatedly, as per the specifications above."

The system uses the Chainlink oracle with the VRF Direct funding method for a source of randomness. An explanation on how the VRF Direct Funding works can be found here.

"The Direct funding method does not require you to create subscriptions and pre-fund them. Instead, you must directly fund consuming contracts with LINK tokens before they request randomness."

"For Chainlink VRF v2 to fulfill your requests, you must have a sufficient amount of LINK in your consuming contract"

But the owner can make the requests made by the oracle purposefully fail by not funding the consuming contract with enough LINK tokens:

"Make sure that your consuming contracts are funded with enough LINK tokens to cover the transaction costs. If the consuming contract doesn’t have enough LINK tokens, your request will revert."

Additionally, the owner can take any opportunity the oracles fails enough times to swap the source as well.

The code for swapping the source only verifies that the caller is the owner, and that enough retries have been made:

File: src/RNSourceController.sol

89:     function swapSource(IRNSource newSource) external override onlyOwner {
90:         if (address(newSource) == address(0)) {
91:             revert RNSourceZeroAddress();
92:         }
93:         bool notEnoughRetryInvocations = failedSequentialAttempts < maxFailedAttempts;
94:         bool notEnoughTimeReachingMaxFailedAttempts = block.timestamp < maxFailedAttemptsReachedAt + maxRequestDelay;
95:         if (notEnoughRetryInvocations || notEnoughTimeReachingMaxFailedAttempts) {
96:             revert NotEnoughFailedAttempts();
97:         }
98:         source = newSource;
99:         failedSequentialAttempts = 0;
100:        maxFailedAttemptsReachedAt = 0;
101:
102:        emit SourceSet(newSource);
103:        requestRandomNumberFromSource();
104:    }

Link to code

Other vector for making the oracle fail could be on the fulfillRandomWords function, as stated on the
documentation:

If your fulfillRandomWords() implementation reverts, the VRF service will not attempt to call it a second time. Make
sure your contract logic does not revert.

File: src/VRFv2RNSource.sol

32:    function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) internal override {
33:        if (randomWords.length != 1) {
34:            revert WrongRandomNumberCountReceived(requestId, randomWords.length); // @audit should not revert
35:        }
36:
37:        fulfill(requestId, randomWords[0]); // @audit-info calls another function that reverts
38:    }

Link to code

File: src/RNSourceBase.sol

33:    function fulfill(uint256 requestId, uint256 randomNumber) internal {
34:        if (requests[requestId].status == RequestStatus.None) {
35:            revert RequestNotFound(requestId); // @audit should not revert
36:        }
37:        if (requests[requestId].status == RequestStatus.Fulfilled) {
38:            revert RequestAlreadyFulfilled(requestId); // @audit should not revert
39:        }
40:
41:        requests[requestId].randomNumber = randomNumber;
42:        requests[requestId].status = RequestStatus.Fulfilled;
43:        IRNSourceConsumer(authorizedConsumer).onRandomNumberFulfilled(randomNumber); // @audit-info calls another function that reverts
44:
45:        emit RequestFulfilled(requestId, randomNumber);
46:    }

Link to code

File: src/RNSourceController.sol

46:    function onRandomNumberFulfilled(uint256 randomNumber) external override {
47:        if (msg.sender != address(source)) {
48:            revert RandomNumberFulfillmentUnauthorized(); // @audit should not revert
49:        }
50:
51:        lastRequestFulfilled = true;
52:        failedSequentialAttempts = 0;
53:        maxFailedAttemptsReachedAt = 0;
54:
55:        receiveRandomNumber(randomNumber);
56:    }

Link to code

Tools Used

Manual review

Recommended Mitigation Steps

Possible mitigations are:

• Implement a proxy that deploys the same version of the VRFv2RNSource contract, and only let the owner swap to a new implementation with new values. This prevents any manipulation, but it has the drawback of fixing the implementation to the current Chainlink Oracle contract.

• Implement contracts for other possible oracles that may be useful integrating in case of a failure on the original oracle. This goes inline with the previous approach.

• Implement a mechanism that cancels any ongoing draw, or sets a timelock to allow users to claim back the tokens for the tickets they bought, until the new source is set. This way it prevents rigging an ongoing draw, but on the other hands it doesn't prevent the owner from getting tokens from the init pot.

[thereksfour]

Unlike code-423n4/2022-12-forgeries-findings#272, the counterparty here is the wenwin protocol rather than an unknown third party, so this is a centralization issue, At the same time, since it already exists in autofind, it should be set to invalid

[c4-judge]

thereksfour marked the issue as unsatisfactory:
Invalid

[c4-judge]

thereksfour marked the issue as primary issue