T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:221
- Finding
- Limit Order May Execute with the Opposite Trade Direction Due to Stale React State<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 221–255 **Vulnerability Type**: Transaction integrity flaw caused by asynchronous state handling **Risk Level**: High ### Vulnerable Code ```tsx const [side, setSide] = useState(OrderSide.BUY); const placeLimitOrder = async () => { const order = { symbol, side, order_type: OrderType.LIMIT, order_price: parseFloat(price), order_quantity: parseFloat(quantity), }; const result = await submitOrder(order); if (result.success) { toast.success('Limit order placed!'); } }; return ( <div> <Input label="Price" value={price} onChange={(e) => setPrice(e.target.value)} /> <Input label="Quantity" value={quantity} onChange={(e) => setQuantity(e.target.value)} /> <Button color="buy" onClick={() => { setSide(OrderSide.BUY); placeLimitOrder(); }} > Buy </Button> <Button color="sell" onClick={() => { setSide(OrderSide.SELL); placeLimitOrder(); }} > Sell </Button> </div> ); ``` ### Technical Analysis React state updates through `setSide()` are asynchronous and are not guaranteed to update the `side` value within the current event-handler invocation. Both button handlers update the state and then immediately invoke `placeLimitOrder()`. That function reads `side` from the existing render closure, which may still contain the previous trade direction. Because the initial state is `OrderSide.BUY`, clicking **Sell** on the initial render can submit a buy order. The inverse can also occur after a prior state transition: clicking **Buy** while the current rendered state is `SELL` may submit a sell order. The order is sent to the financial API through `submitOrder(order)`, so this is not merely a display inconsistency. The stale value directly controls the direction of a potentially real transaction. ### Attack Path 1. The connected and authenticated user open ...[truncated 1680 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Do not use an asynchronously updated state value as the immediate source of authority for a financial transaction. Pass the selected side directly to the submission function: ```tsx const placeLimitOrder = async (selectedSide: OrderSide) => { const parsedPrice = Number(price); const parsedQuantity = Number(quantity); if ( !Number.isFinite(parsedPrice) || parsedPrice <= 0 || !Number.isFinite(parsedQuantity) || parsedQuantity <= 0 ) { toast.error('Enter a valid positive price and quantity.'); return; } const order = { symbol, side: selectedSide, order_type: OrderType.LIMIT, order_price: parsedPrice, order_quantity: parsedQuantity, }; const result = await submitOrder(order); if (result.success) { toast.success('Limit order placed!'); } }; <Button color="buy" onClick={() => placeLimitOrder(OrderSide.BUY)}> Buy </Button> <Button color="sell" onClick={() => placeLimitOrder(OrderSide.SELL)}> Sell </Button> ``` Additional hardening measures: 1. Display a confirmation step containing the exact side, symbol, price, quantity, leverage, and estimated exposure before submission. 2. Disable both submission buttons while an order request is pending to prevent duplicate orders. 3. Validate that price and quantity are finite, positive, within market precision constraints, and within account risk limits. 4. Add automated tests asserting that clicking Buy always submits `OrderSide.BUY` and clicking Sell always submits `OrderSide.SELL`, regardless of prior state. 5. Record and display the order direction returned by the API rather than showing a generic success message. 6. Use the SDK's validated order-entry abstraction where possible and keep transaction-critical values local to the explicit user action. ]]>