DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Invalid

upgrade function is not protected

Summary

upgrade function is not protected

Vulnerability Details

function upgrade(
RootProxy.BranchUpgrade[] memory branchUpgrades,
address[] memory initializables,
bytes[] memory initializePayloads
)
external
{
_authorizeUpgrade(branchUpgrades);
RootUpgrade.Data storage rootUpgrade = RootUpgrade.load();
rootUpgrade.upgrade(branchUpgrades, initializables, initializePayloads);
}

The upgrade function calls _authorizeUpgrade, which is marked with the onlyOwner modifier, the upgrade function itself is not restricted. This means that anyone can call the upgrade function, and only the internal _authorizeUpgrade check is protected.

This is problematic because:

  1. It allows non-owners to initiate the upgrade process, even though they shouldn't be able to pass the authorization check.

  2. It may lead to unnecessary gas consumption as non-owners can cause the contract to execute part of the upgrade logic before reverting.

  3. It could potentially be exploited if there's any side effect in the code before the _authorizeUpgrade check.

The _authorizeUpgrade function is empty, which means it doesn't actually perform any checks on the branchUpgrades parameter.

Impact

It may lead to unnecessary gas consumption as non-owners can cause the contract to execute part of the upgrade logic before reverting.

Tools Used

Manual Review

Recommendations

The upgrade function should also be restricted to the owner

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.