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

Immutable `perpVault` Setting Limiting Upgrade Flexibility


Details:
The contract provides a function setPerpVault which allows the owner to set the perpVault address. However, it is designed to only allow this operation once, as evidenced by the requirement that perpVault must be the zero address before it can be set. This means that after the initial configuration, the owner cannot update or change the perpVault without deploying a new contract.


Root Cause:
The root cause is the design decision implemented in the setPerpVault function where the following check is used:

require(perpVault == address(0), "already set");

This condition enforces that once perpVault is assigned, any further attempts to modify it will revert. If the intention was to have a permanently fixed perpVault, initializing it within the constructor would have been a more transparent approach.


Impact:

  • Upgrade Inflexibility: If the owner needs to update or replace the perpVault (e.g., due to a security issue or an upgrade), they are forced to deploy a new contract.

  • Operational Disruption: Redeploying the contract can be cumbersome and may disrupt ongoing operations or integrations that depend on the current perpVault configuration.

  • Limited Recovery Options: In the event of an error during the initial setup, there is no straightforward mechanism to correct the perpVault address without significant overhead.


Recommendation:

  • Constructor Initialization: If the design goal is to have a fixed perpVault, set it in the constructor. This makes the immutability explicit and avoids any confusion regarding future modifications.

  • Upgradeable Pattern (if flexibility is desired): If there is a possibility that perpVault might need to be updated in the future, implement a secure upgrade mechanism that allows for controlled changes, such as a two-step ownership transfer or an upgrade proxy pattern.


Proof of Concept:

  1. Initial Setup Success:

    • Deploy the contract.

    • Call setPerpVault(newVaultAddress, market) with a valid newVaultAddress and market.

    • The transaction succeeds, and perpVault is set to newVaultAddress.

  2. Attempted Update Failure:

    • Attempt to call setPerpVault(anotherVaultAddress, market) with a different vault address.

    • The transaction reverts with the error message "already set", confirming that the perpVault can only be set once.

This behavior demonstrates that once set, the perpVault cannot be updated, which aligns with the identified issue.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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