Steadefi

Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Submission Details
Severity: medium
Valid

Compound must be executed until it works

Summary

If GMX fails during the compounding process in the addLiquidity function, then processCompoundCancellation is executed, which sets the state to Compound_Failed:

127: function processCompoundCancellation(
128: GMXTypes.Store storage self
129: ) external {
130: GMXChecks.beforeProcessCompoundCancellationChecks(self);
131:
132: self.status = GMXTypes.Status.Compound_Failed;
133:
134: emit CompoundCancelled();
135: }
136: }

In this state, the only function that can be called is compound. This means that compound must be executed repeatedly until it succeeds. Otherwise, the state remains stuck in Compound_Failed.

Vulnerability Details

This is what such a scenario might look like:

  1. There are some TokenA/TokenB in GMXTrove.

  2. A keeper calls the compound function.

  3. The tokens are attempted to be deposited in GMX.

  4. The deposit fails (for example, due to insufficient GM supply for purchase or to high slippage amount).

  5. The status is set to Compound_Failed.

  6. The keeper calls the compound function again, and it fails once more.

Impact

It could render the vault temporarily unusable as it gets stuck during the compound operation until it works.

Tools Used

VSCode

Recommendations

In the processCompoundCancellation function, the TokenA/TokenB should be sent back to GMXTrove, and the vault should be reopened so that other functions can be called. This way, you can still retry the Compound operation, but the vault won't remain stuck in the same state if it fails again and again.

function processCompoundCancellation(
GMXTypes.Store storage self
) external {
GMXChecks.beforeProcessCompoundCancellationChecks(self);
if (self.tokenA.balanceOf(address(this)) > 0) {
self.tokenA.safeTransfer(self.trove, self.tokenA.balanceOf(address(this)));
}
if (self.tokenB.balanceOf(address(this)) > 0) {
self.tokenB.safeTransfer(self.trove, self.tokenB.balanceOf(address(this)));
}
self.status = GMXTypes.Status.Open;
emit CompoundCancelled();
}
}
Updates

Lead Judging Commences

hans Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong status transition on processCompoundCancellation

Impact: High Likelihood: Low The sponsor confirmed it's a typo in the diagram but the documentation/source is the source of truth for the hawks. Will group all findings pointing out the wrong status transition to Compound_Failed based on the diagram.

Support

FAQs

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