# reduceOnly limit order update does not take into account that a new amm may be selected, which may lead to loss of funds Crit/High · Three Sigma · NFT perpetuals · 2nd January, 2024 Finding 3S-NFTPerp-C01 of the Nftperp Exchange security review. - Report: /reports/nftperp-i - Source: https://cdn.sanity.io/files/qoqld077/production/c19530de75e234ad15694b4563edb1fc9d2a3fd8.pdf --- ### Description On a limit order update, in [_updateLimitOrder()](https://github.com/nftperp/NFTPerp-V2-Contracts/blob/main/src/clearinghouse-libs/OrderCreator.sol#L293-L301), changing amm is not being taken into account. The following scenarios are possible: - if reduceOnly remains false, no problem - if reduceOnly goes from false to true, no problem - if reduceOnly remains true, it won't link the reduceOnly order in the new amm B, nor remove it from amm A - if reduceOnly goes from true to false, it will remove the linked order from the new amm B, instead of the old amm A. This seems the worst scenario, as updating the limit order from false to true will pull weth from the trader, but this order may be [deleted](https://github.com/nftperp/NFTPerp-V2-Contracts/blob/main/src/clearinghouse-libs/PositionManager.sol#L245) if the trader closes its position on amm A, losing the funds. ### Recommendation Correctly handle the case when the amm changes. ### Status Addressed in [#6424621](https://github.com/nftperp/NFTPerp-V2-Contracts/commit/6424621b69586065e57c01e71ec107b5c952b69e) --- Related findings: - [OstiumTrading::updateOpenLimitOrder() Makes Unnecessary Field Updates](https://0xsimao.com/findings/ostium-trading-unnecessary-field-updates): Ostium - [Providing liquidity to the AMM does not check the return value of actually provided tokens leading to locked funds.](https://0xsimao.com/findings/cork-protocol-providing-actually-provided-locked): Cork Protocol - [Liquidating maturies with unassigned earnings will not take into account floating assets increase leading to loss of funds](https://0xsimao.com/findings/exactly-protocol-update-staking-contract-ii-liquidating-maturies-unassigned-floating): Exactly Protocol Update - Staking Contract - [OstiumTrading::executeAutomationOrder() and _getLimitOrdersToTrigger() should check isPaused for open limit orders](https://0xsimao.com/findings/ostium-automation-orders-trigger-paused): Ostium