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

Inconsistent totalShares update in _cancelflow function

Title

Inconsistent totalShares update in _cancelflow function

Summary

The _cancelFlow function properly handles some aspects of canceling deposits and withdrawals but misses updating the totalShares variable. This inconsistency can lead to inaccurate tracking of shares and potential economic issues.

Vulnerability Details

The issue lies in the _cancelFlow function within PerpetualVault.sol. When canceling a flow, the function refunds funds and adjusts totalDepositAmount but does not reset totalShares:

PerpetualVault.sol
1220: function _cancelFlow() internal {
1221: if (flow == FLOW.DEPOSIT) {
1222: uint256 depositId = counter;
1223: collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
1224: totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
1225: EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
1226: try IGmxProxy(gmxProxy).refundExecutionFee(
1227: depositInfo[counter].owner,
1228: depositInfo[counter].executionFee
1229: ) {} catch {}
1230: delete depositInfo[depositId];
1231: } else if (flow == FLOW.WITHDRAW) {
1232: try IGmxProxy(gmxProxy).refundExecutionFee(
1233: depositInfo[counter].owner,
1234: depositInfo[counter].executionFee
1235: ) {} catch {}
1236: }
1237:
1238: // Setting flow to liquidation has no meaning.
1239: // The aim is to run FINAIZE action. (swap indexToken to collateralToken);
1240: flow = FLOW.LIQUIDATION;
1241: nextAction.selector = NextActionSelector.FINALIZE;
1242: }
1243:

The function correctly updates totalDepositAmount but doesn't adjust totalShares, leading to mismatched values.

Impact

This oversight causes totalShares to become inaccurate. Since the share balance no longer reflects reality, it can create economic imbalances and inflate the vault's value unintentionally.

Tools Used

Manual Review

Recommendations

Modify the _cancelFlow function to properly adjust totalShares when canceling a deposit or withdrawal. Ensure the update happens before resetting the flow state to maintain accurate tracking.

Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_cancelFlow_can_be_called_after_order_execution_leading_to_disturb_shares_and_refund_too_many_fees

Likelihood: None/Very Low, when the keeper call cancelFlow after an order execution Impact: High, Inflation/deflation of total shares, and too many fees refunded.

Support

FAQs

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