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

Potential Underflow in `PerpetualVault.afterOrderExecution`

Summary

The PerpetualVault.afterOrderExecution function has a potential underflow issue when calculating the increased amount for share minting. This can cause transaction reverts, failed share minting, and locked funds.

Vulnerability Details

When calculating the increased amount for share minting, the function subtracts 1 after deducting fees and price impact. If the total deductions exceed the deposit amount, this results in an underflow, causing the transaction to revert.

Root Cause:
The calculation of the increased amount in the PerpetualVault.afterOrderExecution function can cause an underflow if the total deductions exceed the deposit amount.

Proof of Concept:

  1. Code Analysis:

    uint256 increased;
    if (priceImpact > 0) {
    increased = amount - feeAmount - uint256(priceImpact) - 1;
    } else {
    increased = amount - feeAmount + uint256(-priceImpact) - 1;
    }
  2. Example Scenario:

    • Deposit:

      • User deposits 1,000 tokens.

      • amount = 1,000.

    • Fee and Price Impact Calculation:

      • Assume feeAmount = 500 and priceImpact = 500.

      • Total deductions: feeAmount + priceImpact + 1 = 500 + 500 + 1 = 1,001.

    • Underflow:

      • increased = amount - feeAmount - uint256(priceImpact) - 1 = 1,000 - 500 - 500 - 1 = -1.

      • This causes an underflow, and the transaction reverts.

  3. Unit Test: copy and paste this code into PerpetualVault.t.sol add these:

    import { Order } from "../contracts/libraries/Order.sol";

    import { IGmxProxy } from "../contracts/interfaces/IGmxProxy.sol";

    function test_Underflow_In_AfterOrderExecution() external {
    address keeper = PerpetualVault(vault).keeper();
    address alice = makeAddr("alice");
    depositFixture(alice, 1e10);
    MarketPrices memory prices = mockData.getMarketPrices();
    bytes[] memory data = new bytes[](1);
    data[0] = abi.encode(3380000000000000);
    vm.prank(keeper);
    PerpetualVault(vault).run(true, false, prices, data);
    PerpetualVault.FLOW flow = PerpetualVault(vault).flow();
    assertEq(uint8(flow), 2);
    // Simulate GMX order execution with high fees and price impact to trigger underflow
    uint256 amount = 1e10;
    uint256 feeAmount = amount / 2; // 50% fee
    int256 priceImpact = int256(amount / 2); // 50% negative price impact
    IGmxProxy.OrderResultData memory orderResultData = IGmxProxy.OrderResultData({
    orderType: Order.OrderType.MarketIncrease,
    sizeDeltaUsd: amount,
    isSettle: false,
    isLong: false,
    outputToken: address(0),
    outputAmount: 0
    });
    vm.prank(address(gmxProxy2x1));
    vm.expectRevert(); // Expect the transaction to revert due to underflow
    PerpetualVault(vault).afterOrderExecution(
    bytes32(0),
    bytes32(0),
    orderResultData,
    prices
    );
    }

Impact

Failed share allocation leaves user deposits locked in the contract without corresponding shares, leading to loss of funds.

Tools Used

Manual

Recommendations

To prevent the underflow, ensure that the total deductions do not exceed the deposit amount. One approach is to add a check before performing the subtraction:

uint256 increased;
if (priceImpact > 0) {
require(amount > feeAmount + uint256(priceImpact) + 1, "Underflow risk");
increased = amount - feeAmount - uint256(priceImpact) - 1;
} else {
require(amount > feeAmount + 1, "Underflow risk");
increased = amount - feeAmount + uint256(-priceImpact) - 1;
}
Updates

Lead Judging Commences

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

Give us feedback!