c4-bot-3 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-init-capital-invitational/blob/a01c4de620be98f9e57a60cf6a82d4feaec54f58/contracts/hook/MarginTradingHook.sol#L487

Vulnerability details

Impact

stop loss order and take profits order lacks expiration time

Proof of Concept

uniswap swap has expiration,

https://docs.1inch.io/docs/limit-order-protocol/introduction/#rfq-order-features

1inch limit order and rfq order has expiration time as well

however, the stoploss order and take profits order does not have expiration time

Financial markets are dynamic and conditions can change rapidly. An order that makes sense under certain market conditions might not be appropriate if those conditions change. By setting an expiration time, traders can ensure that their orders are executed only during a time frame when they believe their trading strategy is valid.

Expiration times allow traders to manage risks more effectively. For example, a trader might not want a stop-loss order to remain active indefinitely, as it could get triggered at an undesirable point in the future due to short-term market volatility or overnight market movements.

Traders often align their orders with specific trading strategies or time frames. For instance, a day trader might only want their orders to be active during a single trading session to avoid exposure to overnight market movements.

Without an expiration time, orders could remain open for an extended period, potentially leading to unintended trades. This is particularly relevant in fast-moving markets or for traders who place numerous orders.

Tools Used

Manual Review

Recommended Mitigation Steps

recommend adding expiration time when order is created

https://github.com/code-423n4/2024-01-init-capital-invitational/blob/a01c4de620be98f9e57a60cf6a82d4feaec54f58/contracts/hook/MarginTradingHook.sol#L487

__orders[orderId] = (
    Order({
        initPosId: initPosId,
        triggerPrice_e36: _triggerPrice_e36,
        limitPrice_e36: _limitPrice_e36,
        collAmt: _collAmt,
        tokenOut: _tokenOut,
        orderType: _orderType,
        status: OrderStatus.Active,
        recipient: msg.sender,
        expireAt: expiration // added code here
    })
);

Assessed type

Timing

c4-judge commented 8 months ago

hansfriese marked the issue as primary issue

c4-sponsor commented 8 months ago

fez-init (sponsor) disputed

fez-init commented 8 months ago

It is intended to not support expiration time.