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

`executionFee` is not charged to the first depositors before `positionIsClosed == true` even though the deposits will be processed by the keeper

infect3d

Summary

Before any position is opened, the first users to deposit funds into the vault will get shares directly minted, without the keeper having to open the position into GMX. This will be only once the keeper create the deposit order to GMX that fees will be charged.
The keeper will have to pay for the fees on its own expenses, and all first depositors will have deposited without fees charged.

Vulnerability details

On a first deposit, a vault will have its positionIsClosed state as true, which means the if (positionIsClosed) case will be executed L233-L236, and shares will be minted based on the deposited amount, which no fees charged:

File: contracts/PerpetualVault.sol
215: function deposit(uint256 amount) external nonReentrant payable {
216: _noneFlow();
217: if (depositPaused == true) {
218: revert Error.Paused();
219: }
220: if (amount < minDepositAmount) {
221: revert Error.InsufficientAmount();
222: }
223: if (totalDepositAmount + amount > maxDepositAmount) {
224: revert Error.ExceedMaxDepositCap();
225: }
226: flow = FLOW.DEPOSIT;
227: collateralToken.safeTransferFrom(msg.sender, address(this), amount);
228: counter++;
229: depositInfo[counter] = DepositInfo(amount, 0, msg.sender, 0, block.timestamp, address(0));
230: totalDepositAmount += amount;
231: EnumerableSet.add(userDeposits[msg.sender], counter);
232:
233: if (positionIsClosed) {
234: MarketPrices memory prices;
235: _mint(counter, amount, false, prices);
236: _finalize(hex'');
237: } else {
238: _payExecutionFee(counter, true);
239: // mint share token in the NextAction to involve off-chain price data and improve security
240: nextAction.selector = NextActionSelector.INCREASE_ACTION;
241: nextAction.data = abi.encode(beenLong);
242: }
243: }

After that, the keeper will have to call run() to effectively deposit the funds into GMX either through _runSwap >> _doGMXSwap or _createIncreasePosition which creates a GMX position, and will have to bear the price on its own:

File: contracts/PerpetualVault.sol
290: function run(
291: bool isOpen,
292: bool isLong,
293: MarketPrices memory prices,
294: bytes[] memory metadata
295: ) external nonReentrant {
296: _noneFlow();
297: _onlyKeeper();
298: if (gmxProxy.lowerThanMinEth()) {
299: revert Error.LowerThanMinEth();
300: }
301: flow = FLOW.SIGNAL_CHANGE;
302:
303: if (isOpen) {
304: if (positionIsClosed) {
305: if (_isFundIdle() == false) {
306: revert Error.InsufficientAmount();
307: }
308: if (_isLongOneLeverage(isLong)) {
309: _runSwap(metadata, true, prices); <@ if its a x1 vault
310: } else {
311: (uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
312: _createIncreasePosition(isLong, acceptablePrice, prices); <@ if its a xN vault
313: }

Impact

Keeper will have to pay for GMX deposit fees for the users.

Recommended Mitigation Steps

Charge a fee to the first depositors before a position is opened.

Updates

Lead Judging Commences

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