search

sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

chaduke - LimitOrder._closePosition() Revert for the case of within-range, leading to failure in normal casess. #81

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

chaduke

medium

LimitOrder._closePosition() Revert for the case of within-range, leading to failure in normal casess.

Summary

LimitOrder._closePosition() Revert for the case of within-range, leading to failure in normal casess.

Vulnerability Detail

  1. _closePosition() will check the price range as follows:
 if (price <= _limitOrder.priceLowerThreshold) {
            minFillPrice = 0; // can execute below lower limit price threshold
        } else if (price >= _limitOrder.priceUpperThreshold) {
            minFillPrice = _limitOrder.priceUpperThreshold;
        } else {
            revert FlatcoinErrors.LimitOrderPriceNotInRange(
                price,
                _limitOrder.priceLowerThreshold,
                _limitOrder.priceUpperThreshold
            );
        }

While we expect that when the price is within the range, the execution will go normally. However, the above code will revert even when the price is within the range. In other words, the code will revert when price is normal.

Impact

LimitOrder._closePosition() reverts for the case of within-range, leading to failure in normal casess.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/LimitOrder.sol#L151-L161

Tool used

VCode

Manual Review

Recommendation

Change the logic so that normal price will not revert.

sherlock-admin commented 8 months ago

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

karanctf commented:

interesting

takarez commented:

invalid

rashtrakoff commented 8 months ago

Limit orders are akin to stop loss/profit take strategies. So they need to be executed when the price is not in the range. I can see the custom issue name is a bit misleading as I think it should be named FlatcoinErrors.LimitOrderPriceInRange but the function fulfills its purpose.