0xSpearmint1 - Protocol does not refund excess funds after calling `RngWitnet.startDraw`

[sherlock-admin4]

0xSpearmint1

high

Protocol does not refund excess funds after calling RngWitnet.startDraw

Summary

Protocol does not refund excess funds after calling RngWitnet.startDraw

Vulnerability Detail

The following RngWitnet.startDraw function is a crucial function that must be called a lot of times in a timely manner for the protocol to get random numbers to determine winners.

Users must send in ETH to cover the cost of requesting the random number from Witnet to this function.

function startDraw(uint256 rngPaymentAmount, DrawManager _drawManager, address _rewardRecipient) external payable returns (uint24) {
        (uint32 requestId,,) = requestRandomNumber(rngPaymentAmount);
        return _drawManager.startDraw(_rewardRecipient, requestId);
    }

which forwards it to the requestRandomNumber function

function requestRandomNumber(uint256 rngPaymentAmount) public payable returns (uint32 requestId, uint256 lockBlock, uint256 cost) {
        Requestor requestor = getRequestor(msg.sender);
        unchecked {
            requestId = ++lastRequestId;
            lockBlock = block.number;
        }
        requests[requestId] = lockBlock;
        cost = requestor.randomize{value: msg.value}(rngPaymentAmount, witnetRandomness);

        emit RandomNumberRequested(requestId, msg.sender, rngPaymentAmount, cost);
    }

which forwards it to the requestor.randomize function

function randomize(uint value, IWitnetRandomness _witnetRandomness) external payable onlyCreator returns (uint256) { 
        uint cost = _witnetRandomness.randomize{ value: value }();
        return cost;
    }

if msg.value > cost, the difference will be refunded from Witnet back to this contract

The problem is that the difference is not refunded to the caller instead it stays in the contract, there is no way for the caller to retrieve these funds

To be clear it is not user error to send in funds > estimatedCost

Users can only get an estimate of the fee using the estimateRandomizeFee function, but this is not to be relied on according to Witnet themselves who state that it always overestimates.

If we check how Witnet recommends to implement this we see that they refund the excess to the msg.sender

function getRandomNumber() external payable {
    latestRandomizingBlock = block.number;
    uint _usedFunds = witnet.randomize{value: msg.value}();
    if (_usedFunds < msg.value) {
        payable(msg.sender).transfer(msg.value - _usedFunds);
    }
}

This opens up many attack vectors and general issues

1. Since the extra funds are not refunded, bots will try to call RngWitnet.startDraw by sending the least amount of ETH as possible for the Tx to go through, this will lead to many failed requests due to insufficient ETH.

2. Since users will only send just the amount of ETH they think they need, this opens up a serious griefing attack where an attacker will frontrun users calling RngWitnet.startDraw by manipulating the RandomizeFee so that it raises > msg.value, causing the randomness request to fail.

Impact

Every time RngWitnet.startDraw is called there will be excess funds locked in the contract, this is a huge inefficiency and discourages users from calling the function

The extra funds will not be refunded to the user. This will discourage users/bots from calling RngWitnet.startDraw in a timely manner.

Attacker can cause requests to fail by spiking the gas price just a bit leading to the RngWitnet.startDraw call to revert

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-rng-witnet/src/RngWitnet.sol#L132

Tool used

Manual Review

Recommendation

Refund msg.value - cost to the user who made the call

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

excess funds stays in the requestor contract controlled by the user who can take them back

[nevillehuang]

Invalid, low severity, bots can simply implement logic to send a few wei extra/have logic to refund excess fees