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

Failed Compound causes DOS to the entire system

0xasen

Summary

A failed Compound can cause a DOS to pretty much the entire system. The reason for this is because of the status set in processCompoundCancellation which is the last function called in the user flow where a compound action initiated by the Keeper fails for some reason.

There could be many reasons for this - for example, a lack of liquidity which incurs much bigger slippage than what we want as a system, resulting in failed token swap and failed compound action. The system is expected to handle such cases without bricking other functionality.

Vulnerability Details

Let's see the code of processCompoundCancellation in GMXCompound.sol:

function processCompoundCancellation(GMXTypes.Store storage self) external {
GMXChecks.beforeProcessCompoundCancellationChecks(self);
​
self.status = GMXTypes.Status.Compound_Failed; <--
​
emit CompoundCancelled();
}

As you can see, the status is set as Compound_Failed. I won't get into detail on how we end up using this function as the report will get very long but this is basically the last function that gets called if the keeper tries to "compound" and the compounding fails. The whole flow can be easily checked in the detailed sequence diagrams made by the SteadeFi team.

Setting the status as Compound_Failed bricks the functionality of very important functions like deposit, withdraw, and rebalance.

Check before a deposit:

function beforeDepositChecks(
GMXTypes.Store storage self,
uint256 depositValue
) external view {
if (self.status != GMXTypes.Status.Open)
revert Errors.NotAllowedInCurrentVaultStatus();
​
//other unrelated checks below
}

Check before a withdraw:

function beforeWithdrawChecks(
GMXTypes.Store storage self
) external view {
if (self.status != GMXTypes.Status.Open)
revert Errors.NotAllowedInCurrentVaultStatus();
​
//other unrelated checks below

Check before a rebalance:

function beforeRebalanceChecks(
GMXTypes.Store storage self,
GMXTypes.RebalanceType rebalanceType
) external view {
if (
self.status != GMXTypes.Status.Open &&
//this only gets set later in the process of rebalance so the functionality here will be bricked as well
self.status != GMXTypes.Status.Rebalance_Open
) revert Errors.NotAllowedInCurrentVaultStatus();
//other unrelated checks below
}

As you can see, these primary functions will be completely bricked because the status is not Open. The sponsor confirmed that the keepers will likely try to compound once a day, meaning this DOS can last days at a time making the system basically useless.

The only way to fix the DOS is if we manage to successfully process a compound which sets the status again to open:

function processCompound(
GMXTypes.Store storage self
) external {
GMXChecks.beforeProcessCompoundChecks(self);
​
self.status = GMXTypes.Status.Open;
​
emit CompoundCompleted();
}

However, this might not be possible immediately and the keepers likely won't try to compound again leaving the system DOSed for an undefined amount of time.

Additionally, according to the sequence diagram, it looks like we expect the status to be Open after processCompoundCancellation:
Screenshot

Impact

DOS of the most important functions in the protocol which could continue for days.

That is not intended behavior and will lead to frustrated users and eventually to losses for the protocol.

Tools Used

Manual review

Recommendations

Set the status to open even if the compound failed:
self.status = GMXTypes.Status.Open;

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.