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

Reentrancy Vulnerability in `PerpetualVault._handleReturn`

cadila909

Summary

A reentrancy vulnerability has been identified in the PerpetualVault._handleReturn function. The function makes an external call to _transferToken(depositId, amount) before updating critical state variables. This allows an attacker to re-enter the contract and manipulate its state, potentially leading to fund loss, double withdrawals, or inconsistent balances.

Vulnerability Details

Affected Code (PerpetualVault.sol, lines #1129-1156)

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
uint256 shares = depositInfo[depositId].shares;
uint256 amount;
​
if (positionClosed) {
amount = collateralToken.balanceOf(address(this)) * shares / totalShares;
} else {
uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
amount = withdrawn + balanceBeforeWithdrawal * shares / totalShares;
}
​
if (amount > 0) {
_transferToken(depositId, amount); // ⚠️ External call before state update (reentrancy risk)
}
​
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId); // State update happens here, but it's too late!
​
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee
) {} catch {}
}
}
​
// State variables deleted too late, leaving room for reentrancy
delete swapProgressData;
delete flowData;
delete flow;
}
​

Key Issues

  1. Reentrancy Risk: _transferToken(depositId, amount) makes an external call to a token contract before deleting swapProgressData, flowData, and flow.

  2. State Manipulation: Since the state is not yet updated, an attacker could re-enter the contract and manipulate balances.

  3. Double Withdrawals: By re-entering, an attacker could withdraw multiple times before state updates take effect.

Impact

Fund Drain: Attackers can withdraw more than their actual balance.

  • State Corruption: Attackers can alter contract states (swapProgressData, flowData, flow).

  • Multiple Withdrawals: Users may withdraw the same deposit more than once.

Tools Used

Vs code

Recommendations

Fix 1: Apply the Checks-Effects-Interactions Pattern

Move all state updates before making external calls to eliminate the reentrancy risk.

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
uint256 shares = depositInfo[depositId].shares;
uint256 amount;
​
if (positionClosed) {
amount = collateralToken.balanceOf(address(this)) * shares / totalShares;
} else {
uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
amount = withdrawn + balanceBeforeWithdrawal * shares / totalShares;
}
​
// ✅ First, update contract state to prevent reentrancy
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId);
​
delete swapProgressData;
delete flowData;
delete flow;
​
// ✅ Now, safely execute the external call AFTER updating state
if (amount > 0) {
_transferToken(depositId, amount);
}
​
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee
) {} catch {}
}
}
}
​

Fix 2: Use OpenZeppelin’s ReentrancyGuard

Implement ReentrancyGuard to prevent multiple calls in a single transaction.

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract PerpetualVault is ReentrancyGuard {
function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal nonReentrant {
// Function logic remains the same, but now it's protected from reentrancy
}
}
​

Fix 3: Verify Token Contract Safety

  • Ensure that the ERC20 token used does not have hooks or external callbacks that could be exploited.

  • If possible, use SafeERC20’s safeTransfer instead of direct function calls.

Updates

Lead Judging Commences

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

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

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

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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