search

sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

GoSlang - User not allowed to close orders their own order due to keeper fee #58

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

GoSlang

medium

User not allowed to close orders their own order due to keeper fee

Summary

The protocol always takes a fee for any keeper to execute the announced order, this is to ensure that the executor does not lose funds by executing the user's order and allowing them to interact with the protocol.

Vulnerability Detail

An issue arises when a user wants to close their very small position, the protocol does not check if the executor is the user who owns that position and allows them to bypass the keeper fee, since closing an order requires the keeper fee to be subtracted from the user's margin. there is a case where the user might only have enough to cover the trading fee and wants to close their position due to the funding fees eating away at their position but the user does not have enough to cover the preset keepers fee that they would be paying to them self in this case.

See this code or the snippet below

Impact

User positions can be left in a state where they can't be closed without adding more margin or the user has to wait for the position to be liquidated or gain enough funding fees to be closed

Code Snippet

    totalFee = announcedClose.tradeFee + _order.keeperFee;

    if (settledMargin <= 0) revert FlatcoinErrors.ValueNotPositive("settledMargin");

    if (settledMargin < int256(totalFee)) revert FlatcoinErrors.NotEnoughMarginForFees(settledMargin, totalFee);

Tool used

Manual Review

Recommendation

Check if the user that owns the position is the keeper executing the position and allow them to bypass the fixed keeper fee.

sherlock-admin commented 8 months ago

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

takarez commented:

invalid

nevillehuang commented 8 months ago

Invalid, agree with sponsors comments:

Technically to avoid closure of a position, they can just add more margin or reduce leverage. If adding margin isn't possible and their position doesn't have enough to pay keeper fees then that position isn't worth closing anyway because keeper fees are supposed to be a bit more than the execution cost. I suspect this is even possible given that the position will be liquidatable due to margin buffer requirements exceeding the keeper fee requirements.