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

`PerpetualVault::run` reverts due to insufficient ETH in `GmxProxy` because ETH was not transferred during `PerpetualVault::deposit` when `positionIsClosed == true`

4lifemen

Summary

This protocol uses a keeper architecture to trigger transaction.
Keeper PerpetualVault::run() call will revert if a user deposits when there is no active position positionIsClosed == true because PerpetualVault::deposit didn't pull eth from the user to the GmxProxy contract.

Vulnerability Details

  1. In PerpetualVault::deposit , the function didn't pull eth from user to GmxProxy contract

function deposit(uint256 amount) external nonReentrant payable {
...
​
@> if (positionIsClosed) {
MarketPrices memory prices;
@> _mint(counter, amount, false, prices);
@> _finalize(hex'');
} else {
@> _payExecutionFee(counter, true); // this path pays execution fee
// mint share token in the NextAction to involve off-chain price data and improve security
nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(beenLong);
}
}

  1. Subsequent keeper calls to PerpetualVault::run to deploy fund will revert due to insufficient fund in GmxProxy

function run(
bool isOpen,
bool isLong,
MarketPrices memory prices,
bytes[] memory metadata
) external nonReentrant {
_noneFlow();
_onlyKeeper();
@> if (gmxProxy.lowerThanMinEth()) {
@> revert Error.LowerThanMinEth();
}
...
}

Notably, PerpetualVault::runNextAction() doesn't have this ETH check as it's meant for continuing flows where execution fees were already collected. However, if the initial PerpetualVault::run() can't execute, the protocol can never reach a state where runNextAction() would be called.

Proof of concept

In the PerpetualVault.t.sol::setup(), remove the call to transfer ETH to GmxProxy

function setUp() public {
...
@> // payable(gmxProxy).transfer(1 ether);
...
}

Add this following function in PerpetualVault.t.sol

function test_RunRevertsDueNoExecutionFee() external {
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
​
// Deposit fund into the vault
depositFixture(alice, 1e8);
​
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3380000000000000);
​
vm.prank(keeper);
vm.expectRevert(Error.LowerThanMinEth.selector);
// Kepper calls run function
PerpetualVault(vault).run(true, false, prices, data);
}

Test result:

├─ [21692] TransparentUpgradeableProxy::fallback(true, false, MarketPrices({ indexTokenPrice: PriceProps({ min: 3386164003773116 [3.386e15], max: 3386184003773116 [3.386e15] }), longTokenPrice: PriceProps({ min: 3386164003773116 [3.386e15], max: 3386184003773116 [3.386e15] }), shortTokenPrice: PriceProps({ min: 1000009671490240875000000 [1e24], max: 1000029671490240875000000 [1e24] }) }), [0x000000000000000000000000000000000000000000000000000c021793574000])
│ ├─ [21044] PerpetualVault::run(true, false, MarketPrices({ indexTokenPrice: PriceProps({ min: 3386164003773116 [3.386e15], max: 3386184003773116 [3.386e15] }), longTokenPrice: PriceProps({ min: 3386164003773116 [3.386e15], max: 3386184003773116 [3.386e15] }), shortTokenPrice: PriceProps({ min: 1000009671490240875000000 [1e24], max: 1000029671490240875000000 [1e24] }) }), [0x000000000000000000000000000000000000000000000000000c021793574000]) [delegatecall]
│ │ ├─ [10189] TransparentUpgradeableProxy::fallback() [staticcall]
│ │ │ ├─ [3114] GmxProxy::lowerThanMinEth() [delegatecall]
│ │ │ │ └─ ← [Return] true
│ │ │ └─ ← [Return] true
│ │ └─ ← [Revert] LowerThanMinEth() <@---
│ └─ ← [Revert] LowerThanMinEth() <@---
└─ ← [Return]

Impact

  • Protocol keeper can't deploy fund by calling PerpetualBault::run() due to insufficient ETH in the GmxProxy

  • Protocol must spend its ETH to manually fund GmxProxy

  • Results in protocol ETH loss that should have been covered by user fees

Tools Used

  • foundry

  • manual review

Recommendations

Consider transferring ETH from the user regardless of positionIsClosed

Updates

Lead Judging Commences

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

Support

FAQs

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