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

Risk of Incorrect Inputs in setVaultState()

Summary

The setVaultState() function i the PerpetualVault.sol allows the contract owner to manually modify critical vault parameters. However, if incorrect values are provided (due to human error or misunderstanding), it could result in locked funds, incorrect position management, or system-wide failures. This issue introduces a centralization risk where a single mistake can disrupt operations.

Vulnerability Details

Affected Function

function setVaultState(
FLOW _flow,
uint256 _flowData,
bool _beenLong,
bytes32 _curPositionKey,
bool _positionIsClosed,
bool _isLock,
Action memory _nextAction
) external onlyOwner {
flow = _flow;
flowData = _flowData;
beenLong = _beenLong;
curPositionKey = _curPositionKey;
positionIsClosed = _positionIsClosed;
_gmxLock = _isLock;
nextAction = _nextAction;
}

Issue Details & Potential Consequences

Parameter Potential Incorrect Input Impact
_flow Incorrect flow type (e.g., WITHDRAW instead of DEPOSIT) Users might not be able to withdraw funds properly.
_flowData Invalid numeric value May cause incorrect position calculations.
_beenLong Wrong long/short flag Affects leverage logic & risk management.
_curPositionKey Non-existent position key The contract might manage an invalid GMX position.
_positionIsClosed Setting true while a position is still open Blocks further actions & locks user funds.
_isLock Accidentally set to true Entire system is frozen, stopping swaps, deposits, and withdrawals.
_nextAction Incorrect action data Could trigger wrong trading execution.

Impact

  1. Funds Lockup – Incorrect _isLock or _positionIsClosed values could freeze withdrawals, preventing users from accessing their funds.

  2. Broken Trading Execution – An invalid _curPositionKey may cause the system to manage a non-existent GMX position, leading to failed or incorrect trades.

  3. Protocol Instability – Setting incorrect flow parameters (e.g., _flow, _beenLong) could disrupt risk management and cause unexpected losses or liquidations.

    Severity: High

    • Reason: The bug can disrupt core vault operations, potentially locking user funds, breaking trade execution, or causing protocol instability.

    • Impact: High (affects deposits, withdrawals, and trading logic).

    • Likelihood: High (manual input errors are common without proper validation).

    • Urgency: Immediate fix recommended to prevent system failures and user fund lockups.

Tools Used

Manuel Review

Recommendations

Emit an Event for Transparency

  • Add an event to log all state updates:

event VaultStateUpdated(
FLOW flow,
uint256 flowData,
bool beenLong,
bytes32 curPositionKey,
bool positionIsClosed,
bool isLock,
Action nextAction
);

Add Input Validations

  • Ensure that invalid or conflicting values cannot be entered:

require(positionExists(_curPositionKey), "Invalid position key!");
require(!_isLock || positionIsClosed, "Cannot lock if position is still open!");

Introduce a Time Delay for _gmxLock = true

  • Prevent instant locking to allow recovery:

uint256 public lockActivationTime;
if (_isLock) {
lockActivationTime = block.timestamp + 1 hours; // Delay lock for 1 hour
} else {
_gmxLock = false;
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 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 9 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.

Give us feedback!