sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

lanrebayode77 - Uncapped MaxClaim Increament #99

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

lanrebayode77

medium

Uncapped MaxClaim Increament

Summary

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L85-L88 Despite having trusted admin, it is still necessary to have some admin previledged in check to prevent distrust from some technical users who might seek deeper understanding of how the protocol works.

In OracleFactory.sol, onlyOwner has the previledge of updateing the maxClaim, while this might be important, I believe there should be a no go limit even by the factory deployer who is presumed to be trusted.

This uncapped upgrade leaves a possibility to steal value by increasing the maxClaim to the highest limit

Vulnerability Detail

Check summary.

Impact

In order to claim incentive token, checks in place include;

  if (amount.gt(maxClaim)) revert OracleFactoryClaimTooLargeError();
  if (!factories[IOracleProviderFactory(msg.sender)]) revert OracleFactoryNotRegisteredError();

all onlyOwner needs to do is to increase the maxClaim amount and register a fraudulent OracleProvider that will make claims to exorbitant reward incentive token for a single price update.

The ultimate impact to this is that informed/technical users could see this a way the protocol could stylishly steal value while appearing fair.

Code Snippet

  function updateMaxClaim(UFixed6 newMaxClaim) external onlyOwner {
        maxClaim = newMaxClaim;
        emit MaxClaimUpdated(newMaxClaim);
    }

Tool used

Manual review

Recommendation

Consider having a constant variable that must not be exceeded, with that, it is open to everyone what the maxClaim could be set to in worst case scenario.

sherlock-admin commented 1 year ago

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

141345 commented:

d

darkart commented:

Admin is Trusted so invalid

panprog commented:

invalid because admin is trusted and has many other possibilities to steal funds