imkapadia - Lack of validation of `priceLowerThreshold` and `priceUpperThreshold` value

[sherlock-admin]

imkapadia

medium

Lack of validation of priceLowerThreshold and priceUpperThreshold value

Summary

Lack of TP/SL value check against current price may leads user cost unintended loss.

Vulnerability Detail

priceLowerThreshold and priceUpperThreshold in announceLimitOrder() allows trader to set TP/SL. But it lacks the price validation against current price. It fails to verify whether the TP price is greater than the current market price and whether the SL price is less than the current market price. Consequently, traders may unknowingly set TP and SL prices that cause them to close position in lesser profit or loss only because protocol does not have simple check.

Additionally, the comment provided by the developer, indicating that the function is currently only used for closing existing orders. So, there are chances that function can also be used to set TP/SL when opening the positions.

/// @dev Currently can only be used for closing existing orders

It is noted that it is completely trader's mistakes. But no trader would do this intentionally. It always happens by mystically.

Impact

Position could be close in lesser profit than expected or in loss.

Code Snippet

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

Tool used

Manual Review

Recommendation

(uint256 price, ) = IOracleModule(vault.moduleAddress(FlatcoinModuleKeys._ORACLE_MODULE_KEY)).getPrice();
if (price <= priceLowerThreshold) revert();
else if(price >= priceUpperThreshold) revert();

[sherlock-admin]

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

takarez commented:

invalid

[nevillehuang]

Invalid, users are responsible for their own thresholds when announcing orders, this is purely a sanity check