# OstiumTradingCallbacks::executeAutomationOpenOrderCallback() opens limit orders at market price instead of the limit price Low/Info · Three Sigma · RWA perpetuals DEX · 19th February, 2024 Finding 3S-OS-L07 of the Ostium security review. - Protocol: https://www.ostium.com/ - Report: /reports/ostium - Source: https://cdn.sanity.io/files/qoqld077/production/a95b9c69e0f65d1d6b0e649f0d62a362358ca8ce.pdf --- ### Description Limit orders usually trade at the specified price; however, [OstiumTradingCallbacks::executeAutomationOpenOrderCallback()](https://github.com/0xOstium/smart-contracts-threeSigma/blob/audit-feedback/src/OstiumTradingCallbacks.sol#L290) registers the limit order t.openPrice at priceAfterImpact, which would be different than the specified. For example, a limit order at price 5000 will be opened when the price after impact is below it, for example 4950, and the trade will have an open price of 4950, not 5000 as the limit price specified. This is more profitable for the user as it's buying cheaper, but most likely not the intended behaviour. ### Recommendation If the o.orderType == IOstiumTradingStorage.OpenOrderType.LIMIT, the open price should be o.targetPrice. If o.orderType == IOstiumTradingStorage.OpenOrderType.STOP, the open price should be priceAfterImpact. Note that this would change the [negative pnl](https://github.com/0xOstium/smart-contracts-threeSigma/blob/audit-feedback/src/OstiumTradingCallbacks.sol#L280) of the trade so that should also be adjusted. ### Status Acknowledged --- Related findings: - [`Market::liquidate()` will not work when most of the liquidity is borrowed due to wrong liquidator `transferFrom()` order](https://0xsimao.com/findings/exactly-protocol-update-staking-contract-liquidate-borrowed-liquidator-transfer): Exactly Protocol Update - Staking Contract - [reduceOnly limit order update does not take into account that a new amm may be selected, which may lead to loss of funds](https://0xsimao.com/findings/nftperp-i-reduce-account-amm-selected): Nftperp Exchange - [DoSed withdrawals due to `ManagedLeveragedVault::executeWithdrawalEpoch()` repaying debt above limit](https://0xsimao.com/findings/beraborrow-i-withdrawals-withdrawal-epoch-debt): Beraborrow Managed Dens - [_match() always checks the trigger of the first order of a certain tick, instead of checking i order](https://0xsimao.com/findings/nftperp-ii-match-certain-tick-checking): Nftperp Matching Engine