code-423n4 / 2023-03-wenwin-findings

1 stars 1 forks source link

swapSource() rug pull risk #325

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

After 3 failures the owner can set the source directly, if it is a malicious source can execute rug

Proof of Concept

In order to prevent Chainlink VRF can no longer obtain random numbers for some reason, the protocol provides a retry mechanism. For example, you can set the number of retries: 3, each interval: 1 hour, after 3 times the owner can switch source by swapSource(), using the new source to provide random numbers.

There are several possible reasons for the failure of obtaining random numbers (not limited to the following 2 cases).

  1. Chainlink's own reasons
  2. VRFv2RNSource Link Token balance is insufficient (owner malicious transfer less or not, the user may not find and transfer link token in time) etc.

If this happens, about 3 hours, the owner can modify the source, and the new modified source immediately take effect

    function swapSource(IRNSource newSource) external override onlyOwner {
        if (address(newSource) == address(0)) {
            revert RNSourceZeroAddress();
        }
        bool notEnoughRetryInvocations = failedSequentialAttempts < maxFailedAttempts;
        bool notEnoughTimeReachingMaxFailedAttempts = block.timestamp < maxFailedAttemptsReachedAt + maxRequestDelay;
        if (notEnoughRetryInvocations || notEnoughTimeReachingMaxFailedAttempts) {
            revert NotEnoughFailedAttempts();
        }
        source = newSource;  //<-------set new source
        failedSequentialAttempts = 0;
        maxFailedAttemptsReachedAt = 0;

        emit SourceSet(newSource);
        requestRandomNumberFromSource();
    }

If the owner sets a malicious source, it is very easy to rug all of Lottery's funds.(free to specify jackpot number).

So some kind of mechanism needs to be established to restrict direct modification of source Such as the following mechanism:

  1. The owner can propose a new source, but it will take one week to take effect.
  2. Lottery NFT owners (representing users who have bought tickets) can vote during the week, and if the percentage of votes exceeds 10% of totalSupply, the owner's proposal can be rejected.
  3. After one week, if no one votes or less than 10%, the new source will be effective.

This is not 100% ensure safety, but it can reduce the possibility of malicious rug.

Tools Used

Recommended Mitigation Steps

  1. The owner can propose a new source, but it will take one week to take effect.
  2. Lottery NFT owners (representing users who have bought tickets) can vote during the week, and if the percentage of votes exceeds 10% of totalSupply, the owner's proposal can be rejected.
  3. After one week, if no one votes or less than 10%, the new source will be effective.
c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Invalid