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

_settle() Not Ensuring Correct Collateral Deduction Before runNextAction() in KeeperProxy.sol

Summary

The function runNextAction() in KeeperProxy.sol executes the next action on the PerpetualVault. However, before executing this, it should validate whether _settle() has deducted the correct pending fees (such as borrowing fees or funding fees).

If _settle() does not deduct the required collateral before running the next action, the next action could be executed on incorrect position balances, leading to unexpected liquidations or incorrect fund allocations.

Vulnerability Details

Step 1: Assume a Trader Has an Open Position

  • A trader opens a long position with 10 ETH collateral at $2,000 per ETH.

  • The GMX funding fee mechanism is reducing the collateral over time (e.g., 0.5 ETH in fees).

Initial Position Value
Collateral 10 ETH
Position Size 10 ETH * $2,000 = $20,000
Fees Accumulated 0.5 ETH deducted over time

At this point, the actual collateral available is only 9.5 ETH, but the system still assumes 10 ETH due to incorrect settlement.

Step 2: The Trader Requests to Close the Position

  1. The system calls _settle(), which should deduct the pending 0.5 ETH in fees.

  2. _settle() incorrectly sets initialCollateralDeltaAmount = 1 (instead of the actual pending fees of 0.5 ETH).

  3. As a result, the GMX order does not deduct the full required collateral.

Now, the system incorrectly believes the trader has 10 ETH in collateral instead of the actual 9.5 ETH.

Step 3: runNextAction() Executes Without Checking Pending Fees

Next, runNextAction() executes the withdrawal action without validating if _settle() properly deducted the fees.

function runNextAction(address perpVault, MarketPrices memory prices, bytes[] memory _swapData) external onlyKeeper {
_validatePrice(perpVault, prices); // ✅ Validates price but does NOT check pending fees!
IPerpetualVault(perpVault).runNextAction(prices, _swapData);
}
  • Since _settle() incorrectly deducted only 1 token (instead of 0.5 ETH), the system thinks the trader has 10 ETH, but in reality, they only have 9.5 ETH.

  • This causes an overestimation of available funds, leading to liquidation risk or incorrect fund calculations.

Impact

  • The trader could get liquidated unexpectedly if they attempt to withdraw more than available collateral.

  • The protocol could end up covering the missing 0.5 ETH, which reduces reserve funds.

  • Future traders could get incorrect position sizes due to a miscalculated collateral balance.

Tools Used

Manual Review

Recommendations

Modify runNextAction() in KeeperProxy.sol:

function runNextAction(address perpVault, MarketPrices memory prices, bytes[] memory _swapData) external onlyKeeper {
_validatePrice(perpVault, prices);
// NEW FIX: Check if `_settle()` has properly deducted collateral before proceeding
uint256 requiredCollateral = vaultReader.getNegativeFundingFeeAmount(
IPerpetualVault(perpVault).curPositionKey(), // Get the position key
prices // Market prices
);
// Check if there is enough collateral AFTER `_settle()` execution
if (requiredCollateral > IERC20(IDataStore(perpVault).COLLATERAL_TOKEN()).balanceOf(perpVault)) {
revert Error.InsufficientCollateral(); // 🚨 Prevents incorrect execution
}
// Now it's safe to run the next action
IPerpetualVault(perpVault).runNextAction(prices, _swapData);
}
Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

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.