Malicious users gas grief keepers with limit orders

Summary

The protocol allows users to create limit orders that can be executed when a price reaches a threshold set by the user but the same time restraint set in DelayedOrder is not set for limit orders.

Vulnerability Detail

A problem arises since the owner of the limit order, can cancel the order at any point since the limit orders do not adhear to the same time restraint that market orders do, consider the following case:

UserA creates a limit order The price goes within the position range UserB then wants the execute the userAs order UserA front runs userBs TX and cancels the order Leaving UserB with a failed TX and a loss of the gas expenditure

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LimitOrder.sol#L87-L97

Impact

Code Snippet

    function cancelLimitOrder(uint256 tokenId) external {
        address positionOwner = _checkPositionOwner(tokenId);
        _checkLimitCloseOrder(tokenId);

        delete _limitOrderClose[tokenId];// Delete the limit order

        ILeverageModule(vault.moduleAddress(FlatcoinModuleKeys._LEVERAGE_MODULE_KEY)).unlock(tokenId);

        emit FlatcoinEvents.OrderCancelled({account: positionOwner, orderType: FlatcoinStructs.OrderType.LimitClose});
    }

Tool used

Manual Review

Recommendation

Enforce the same time restraint that is on announced orders to not allow for this to be done within a short time frame.

sherlock-admin commented 8 months ago

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

takarez commented:

invalid: user looses gas too

nevillehuang commented 8 months ago

Seems to be duplicate of #202, but front-running is a non issue on base given comments in #49. Additionally, worse case scenario user B as keeper loses gas fees, user A leaves with order not executed, no incentive for him.

sherlock-audit / 2023-12-flatmoney-judging

GoSlang - Malicious users gas grief keepers with limit orders #61