Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

No Enforcement of Admin-Specified Limits & Lack of Input Validation for `minDepositAmount` & `maxDepositAmount` in `PerpetualVault.sol::initialize()`

affanimran1303

Vulnerability Details:

The initialize() function in the PerpetualVault.sol contract is responsible for setting up the initial state of the vault. Two critical parameters of this function, minDepositAmount and maxDepositAmount, are set through input values.


However, the function initialize() is missing the following implementations:

  1. The protocol documentation specifies that minDepositAmount should be 1000 and maxDepositAmount should be 100000. However, these values are not declared/assigned in the code.

    uint256 public maxDepositAmount; // max cap of collateral token that can be managrable in vault
    uint256 public minDepositAmount; // min deposit amount of collateral

  2. The initialize() function does not validate the values of minDepositAmount and maxDepositAmount. This means the parameters minDepositAmount & maxDepositAmountcan be set to 0any other arbitrary value.

  3. The values of the parameter minDepositAmountmust not be greater than maxDepositAmount

Impact:

  • If minDepositAmount is not set to 1000 as defined in documentations, users can deposit very small amounts, which might not align with the protocol's intended use.

  • If maxDepositAmount is not limited to 100000, the vault could accept deposits beyond the intended limit, potentially leading to liquidity issues or mismanagement of funds.

  • If minDepositAmount is set to 0, users can deposit any amount (even 0), which could lead to a useless or malicious vault.

  • If maxDepositAmount is set to 0, no deposits will be allowed, making the vault unusable.

Recommended Mitigations:

Modify and make an addition to the initialize() function to perform a check and validate the values of both parameters minDepositAmount& maxDepositAmount within the limits (as defined in the documentation).

//Rest of the code
​
require(_minDepositAmount >= 1000, "minDepositAmount must be at least 1000");
require(_maxDepositAmount <= 100000, "maxDepositAmount must be at most 100000");
require(_minDepositAmount <= _maxDepositAmount, "minDepositAmount cannot exceed maxDepositAmount");
​
//Rest of the code

Applying this fix will prevent vault misconfiguration and ensure proper operation considering the value limitations declared in the documentation.

Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Admin is trusted / Malicious keepers

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. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."

n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Admin is trusted / Malicious keepers

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. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."

Support

FAQs

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