santipu_ - Elevated Keeper Fees Result in Forced Liquidations for Traders

[sherlock-admin2]

santipu_

high

Elevated Keeper Fees Result in Forced Liquidations for Traders

Summary

In periods of market volatility, the requirement for a high keeper fee in the Flat Money protocol can prevent traders with minimal margins from closing their positions, leading to unintended liquidations.

Vulnerability Detail

The Flat Money protocol mandates that users set a keeper fee to compensate order executors. This fee must meet or exceed the current Base gas price-derived fee at the time of order announcement. The function _prepareAnnouncementOrder() enforces this by comparing the user's specified keeper fee against the minimum required fee, rejecting any order where the keeper fee is insufficient.

    function _prepareAnnouncementOrder(uint256 keeperFee) internal returns (uint64 executableAtTime) {
        // ...

>>      if (keeperFee < IKeeperFee(vault.moduleAddress(FlatcoinModuleKeys._KEEPER_FEE_MODULE_KEY)).getKeeperFee())
>>          revert FlatcoinErrors.InvalidFee(keeperFee);

        // ...
    }

This becomes problematic during market volatility when the keeper fee can surge, potentially exceeding a trader's available margin. The announceLeverageClose() function highlights this issue by checking if the trader's settled margin can cover the combined trade and keeper fees, leading to a possible forced liquidation if the fees are unaffordable.

uint256 totalFee = tradeFee + keeperFee;
if (settledMargin < int256(totalFee)) revert FlatcoinErrors.NotEnoughMarginForFees(settledMargin, totalFee);

Impact

The escalating keeper fees in volatile markets can trap leveraged traders, preventing position closures and pushing them towards forced liquidations due to insufficient margins to cover the fees.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/DelayedOrder.sol#L640-L641

Tool used

Manual Review

Recommendation

To address this, it's recommended to allow setting a keeper fee below the gas price-derived fee during volatile periods. This adjustment would enable traders to execute orders themselves without incurring keeper fees, thus reducing the risk of liquidation in volatile markets.

    function _prepareAnnouncementOrder(uint256 keeperFee) internal returns (uint64 executableAtTime) {
        // Settle funding fees to not encounter the `MaxSkewReached` error.
        // This error could happen if the funding fees are not settled for a long time and the market is skewed long
        // for a long time.
        vault.settleFundingFees();

-       if (keeperFee < IKeeperFee(vault.moduleAddress(FlatcoinModuleKeys._KEEPER_FEE_MODULE_KEY)).getKeeperFee())
-           revert FlatcoinErrors.InvalidFee(keeperFee);

        // If the user has an existing pending order that expired, then cancel it.
        cancelExistingOrder(msg.sender);

        executableAtTime = uint64(block.timestamp + vault.minExecutabilityAge());
    }

[sherlock-admin]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid: when the price surge; thats believe to be a market condition and user must be so well aware that a healthy margin should be there to compensate in case of that.

[nevillehuang]

Invalid, users should respect and pay current keepers fee during volateile markets by increasing margin to perform associated closures. There is no "issue" here, keeper fees are required to incentivized keepers from executing orders, even for self-executed orders, that is simply the mechanism of the protocol.