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

Unvalidated Swap Metadata Enables Arbitrary Contract Execution in PerpetualVault.sol

jefextar

Summary

The _runSwap function and Paraswap/GMX swap execution mechanisms in PerpetualVault do not validate or sanitize metadata passed from off-chain scripts. This flaw allows attackers to inject malicious router addresses or call data, enabling unauthorized contract calls that can steal funds or manipulate the vault's state.

Vulnerability Details

  • Unvalidated External Metadata: The _runSwap function uses metadata from an off-chain script to execute swaps. This metadata includes PROTOCOL, router address, and callData, which are not validated against a trusted allowlist.

  • Arbitrary Contract Calls: The ParaswapUtils.swap function directly calls the to address (from metadata) with arbitrary callData, allowing attackers to redirect funds to malicious contracts.

  • No Slippage Checks: Swap metadata lacks minOutputAmount, enabling front-running or malicious contracts to drain funds without slippage protection.

Impact

This vulnerability allows attackers to:

  1. Steal Funds: Inject a malicious contract address in swap metadata to redirect assets.

  2. Execute Reentrancy Attacks: Exploit the unguarded swap function to reenter critical vault functions (e.g., withdraw).

  3. Manipulate State: Trigger malicious logic during swaps to circumvent protocol invariants.

Tools Used

  • Static Analysis: Manual code review identified metadata validation gaps in core swap functions.

  • Test Cases: Foundry-based tests simulated malicious metadata injection and fund redirection.

Recommendations

  1. Whitelist Routers: Create a trusted list of router addresses (e.g., Paraswap, GMX) and validate to addresses in swap metadata against this list.

    mapping(address => bool) public trustedRouters;
    function _doDexSwap(bytes memory data) internal {
    (address to, ...) = abi.decode(data, ...);
    require(trustedRouters[to], "TRUSTED_ROUTER_ONLY");
    // Proceed with swap
    }

  2. Enforce Slippage Checks: Require swap metadata to include minOutputAmount and enforce it in _doDexSwap/_doGmxSwap.

  3. Upgrade Swap Libraries: Use audited versions of ParaswapUtils with reentrancy guards and callback validation.

  4. Add Reentrancy Guards: Wrap swap functions with nonReentrant modifiers from OpenZeppelin to prevent recursive attacks.

Example Fix:

--- a/PerpetualVault.sol
+++ b/PerpetualVault.sol
@@ -123,6 +123,7 @@
function _runSwap(...) internal {
(PROTOCOL _protocol, bytes memory data) = abi.decode(metadata[0], ...);
if (_protocol == PROTOCOL.DEX) {
+ require(trustedRouters[router], "TRUSTED_ROUTER_REQUIRED");
ParaSwapUtils.swap(router, data);
}
}

Enables direct fund theft and state manipulation.

Mitigation

Deploy the recommended fixes and test them against adversarial scenarios to ensure metadata validation and fund safety.z

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.

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 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.

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.