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

Unsafe Native Token Transfer in PerpetualVault - DoS Risk Due to Gas Limits

sempermutans

Summary

The PerpetualVault contract directly transfers native tokens using .transfer(), which enforces a fixed gas limit of 2300 gas. Due to and future gas repricing updates, .transfer() can fail unexpectedly, breaking contract execution and potentially causing funds to be locked.

Vulnerability Details

The _payExecutionFee function in PerpetualVault.sol uses the deprecated .transfer() method:

function _payExecutionFee(uint256 depositId, bool isDeposit) internal {
uint256 minExecutionFee = getExecutionGasLimit(isDeposit) * tx.gasprice;
​
if (msg.value < minExecutionFee) {
revert Error.InsufficientAmount();
}
if (msg.value > 0) {
payable(address(gmxProxy)).transfer(msg.value);
depositInfo[depositId].executionFee = msg.value;
}
}

The .transfer() method has a hard-coded gas limit of 2300 gas. This limitation was originally intended as a security measure to prevent reentrancy attacks. However, this approach has become problematic with network upgrades like EIP 1884
which increased the gas costs for certain operations. If the receiving contract (in this case, gmxProxy) has a fallback function that requires more than 2300 gas to execute, the transfer will fail.

Impact

  • Transaction failure if the receiver contract's fallback function requires more than 2300 gas.

  • Potential loss of funds if the transaction cannot be retried.

  • Deposits or withdrawals could be permanently blocked if the .transfer() consistently fails.

  • The contract may become completely unusable if the GMX proxy is upgraded to include more complex fallback logic.

  • Severity: 🟡 Medium

Tools Used

Manual code review

Recommendations

Use low-level calls instead of .transfer() to avoid the 2300 gas limit:

function _payExecutionFee(uint256 depositId, bool isDeposit) internal {
uint256 minExecutionFee = getExecutionGasLimit(isDeposit) * tx.gasprice;
​
if (msg.value < minExecutionFee) {
revert Error.InsufficientAmount();
}
if (msg.value > 0) {
(bool success, ) = address(gmxProxy).call{value: msg.value}("");
if (!success) revert Error.TransferFailed();
depositInfo[depositId].executionFee = msg.value;
}
}

This approach:

  1. Uses .call{value: amount}("") which forwards all available gas

  2. Properly checks the return value to ensure the transfer succeeded

  3. Reverts with a clear error message if the transfer fails

Proof of Concept for Unsafe Native Token Transfer

Overview:

This proof of concept demonstrates how the use of .transfer() with its fixed 2300 gas limit can cause transactions to fail when the receiving contract needs more gas to execute its fallback function.

Actors:

  • User: A user attempting to deposit funds into the vault.

  • PerpetualVault: The contract using an unsafe transfer method.

  • GMX Proxy: A proxy contract with complex operations in its fallback function.

Working Test Case:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../src/PerpetualVault.sol";
import "../src/mocks/ComplexGmxProxy.sol"; // Mock GMX proxy with a gas-intensive fallback
​
contract UnsafeTransferTest is Test {
PerpetualVault vault;
ComplexGmxProxy gmxProxy;
address user = address(0x1);
function setUp() public {
// Deploy a GMX proxy with gas-intensive fallback function
gmxProxy = new ComplexGmxProxy();
// Initialize vault with required parameters
vault = new PerpetualVault();
vault.initialize(
address(0), // market (mock address)
address(this), // keeper
address(this), // treasury
address(gmxProxy), // gmxProxy
address(0), // vaultReader (mock address)
100, // minDepositAmount
10000, // maxDepositAmount
10000 // leverage
);
// Fund user account
vm.deal(user, 10 ether);
}
function testTransferFailure() public {
// User tries to deposit with execution fee
vm.startPrank(user);
// This will fail because .transfer() only provides 2300 gas
// and the ComplexGmxProxy's fallback function requires more
vm.expectRevert();
vault.deposit{value: 0.1 ether}(1000);
vm.stopPrank();
// Now modify the vault to use .call() instead
// (This would be a manual modification for testing)
PerpetualVaultFixed vaultFixed = new PerpetualVaultFixed();
vaultFixed.initialize(
address(0), // market
address(this), // keeper
address(this), // treasury
address(gmxProxy), // gmxProxy
address(0), // vaultReader
100, // minDepositAmount
10000, // maxDepositAmount
10000 // leverage
);
// User tries again with the fixed contract
vm.startPrank(user);
// This should succeed because .call() forwards all available gas
vaultFixed.deposit{value: 0.1 ether}(1000);
vm.stopPrank();
// Verify the execution fee was properly transferred
assertEq(address(gmxProxy).balance, 0.1 ether);
}
}
​
// Mock contract with the fix implemented
contract PerpetualVaultFixed is PerpetualVault {
// Override the internal function to use .call()
function _payExecutionFee(uint256 depositId, bool isDeposit) internal override {
uint256 minExecutionFee = getExecutionGasLimit(isDeposit) * tx.gasprice;
​
if (msg.value < minExecutionFee) {
revert Error.InsufficientAmount();
}
if (msg.value > 0) {
(bool success, ) = address(gmxProxy).call{value: msg.value}("");
require(success, "Transfer failed");
depositInfo[depositId].executionFee = msg.value;
}
}
}
​
// Mock ComplexGmxProxy that uses more than 2300 gas in its fallback
contract ComplexGmxProxy {
uint256[] private data;
// Gas-intensive fallback function
receive() external payable {
// Do some gas-intensive operations
for (uint i = 0; i < 10; i++) {
data.push(i);
}
}
}

Explanation:

  1. Setup: We create a ComplexGmxProxy contract with a gas-intensive fallback function that requires more than 2300 gas to execute.

  2. Failure Scenario:

    • When a user attempts to deposit funds using the original PerpetualVault contract, the execution fails because .transfer() only forwards 2300 gas.

    • The ComplexGmxProxy's fallback function requires more gas to complete its operations.

  3. Fixed Scenario:

    • We implement a fixed version of the vault that uses .call() instead of .transfer().

    • The same deposit operation succeeds because .call() forwards all available gas.

  4. Verification:

    • We verify that the ether was properly transferred to the GMX proxy contract.

This demonstrates that using .transfer() with its fixed gas limit can cause legitimate transactions to fail under certain conditions, particularly when the receiving contract has a complex fallback function.

Updates

Lead Judging Commences

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

Give us feedback!