Miners or Validators can potentially manipulate randomness generation when maxRequestDelay is low

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/RNSourceController.sol#L60-L75

Vulnerability details

Impact

Manipulation of randomness.

Proof of Concept

While executing the draw, requestRandomNumberFromSource() is called through requestRandomNumber() method which further makes a call on source requesting the random number.

Once the request has been made, the VRF service waits for writing a fulfillment untill a safe block confirmation time frame (_requestConfirmations). The _requestConfirmations is supposed to be set in such a way that potential rewrite attacks becomes unprofitable for miners or validators [which means on a higher time frame side].

Can read about it here: Chainlink VRF Docs

During this period, the Request status will be pending.

File: RNSourceController.sol

  function retry() external override {
        if (lastRequestFulfilled) {
            revert CannotRetrySuccessfulRequest();
        }
        if (block.timestamp - lastRequestTimestamp <= maxRequestDelay) {
            revert CurrentRequestStillActive();
        }

        uint256 failedAttempts = ++failedSequentialAttempts;
        if (failedAttempts == maxFailedAttempts) {
            maxFailedAttemptsReachedAt = block.timestamp;
        }

        emit Retry(source, failedSequentialAttempts);
        requestRandomNumberFromSource();
    }

Link to code

Here's how Miners or Validators can potentially manipulate randomness generation when maxRequestDelay is low using retry method:

1. Once the request has been sent to VRF Service, it will wait untill safe _requestConfirmations.
2. Till then if maxRequestDelay is low, anyone can call the retry method where as the request is still pending it will clear first revert check and second as well because of low maxRequestDelay value.
3. Next, It will send the request again.

Here, as per the Chainlink docs, if the requests of randomness: A, B, C are in short succession, there is no guarantee that the associated randomness fulfillments will also be in order A, B, C. The randomness fulfillments might just as well arrive in order C, A, B or any other order. Blockchain miners/validators can control the order in which the requests appear on-chain.

This is also the reason why chainlink suggests not to re-request randomness in their docs here.

This way miners/validators can control the result through controlling the randomness. They cannot determine the random value in advance. But It enables them to get a fresh random value that might or might not be to their advantage.

Tools Used

VS Code

Recommended Mitigation Steps

Make sure that maxRequestDelay is always set atleast multiple times of the average safe _requestConfirmations time.

[c4-judge]

thereksfour marked the issue as unsatisfactory:
Overinflated severity

[c4-judge]

thereksfour marked the issue as duplicate of #445

[c4-judge]

thereksfour marked the issue as satisfactory

[c4-judge]

thereksfour changed the severity to QA (Quality Assurance)

[c4-judge]

thereksfour marked the issue as grade-c