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

1 stars 1 forks source link

Malicious onlyOwner and changing of Oracle can drain Lottery #70

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

[M-02] Malicious onlyOwner and changing of Oracle can drain Lottery

If RNSourceController's owner is stolen and Chainlink's Oracle is not answering to requests anymore, a hacker can use the owner private key to swap Chainlink's Oracle provider with a hacked implementation, which provides whatever number the hacker would want.

Proof of concept:

contract OracleByHacker {
    uint256 private _requestId;
    function requestRandomnessFromUnderlyingSource() internal override returns (uint256 requestId) {
        requestId = _requestId++;
        emit HackerShouldFulfillRequest(requestId);
    }

    function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) public override ONLY_HACKER {
        fulfill(requestId, randomWords[0]);
    }
}

The above contract would make it so that requests can always be fulfilled (manually by hacker or using an off-chain event listener), which would make it impossible for RNSourceController contract to swap oracles back with a non-malicious one.

Now, the hacker in control could buy tickets, trigger the draw execution each week and supply the exact number they picked, winning the jackpot every time, and effectively draining the Lottery of all funds.

Given all assets are at indirect risk, with a hypothetical attack path with stated assumptions but external requirements, I put the above as a Mid Vulnerability, thinking the developers should take this into consideration and have a remediation plan, given the lottery aims to be a fully decentralized, uninterrupted and non-error prone lottery.

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Invalid