T09 · Insecure Skill Coding Practices
Error
- Location
market_maker.py:202- Finding
Open-order API failures can cause duplicate live orders
- Content
View full analysis
set: """Return set of order IDs Simmer considers open (GTC/GTD on CLOB).""" try: resp = get_client().get_open_orders() return {o["order_id"] for o in resp.get("orders", []) if o.get("order_id")} except Exception as e: print(f" [warn] Could not fetch open orders: {e}") return set() ``` The returned empty set is subsequently used as proof that recorded orders are no longer open: ```python bid_needs_place = post_bid and ( not market_state.get("bid_order_id") or market_state.get("bid_order_id") not in open_order_ids ) ask_needs_place = post_ask and ( not market_state.get("ask_order_id") or market_state.get("ask_order_id") not in open_order_ids ) ``` ### Technical Analysis The implementation does not distinguish between: 1. A successful API response confirming that there are no open orders. 2. A failed API request for which the state of existing orders is unknown. On any SDK, network, authentication, rate-limit, parsing, or remote-service error, `get_open_order_ids()` returns an empty set. Existing locally recorded order IDs then appear absent from the open-order set, causing `bid_needs_place` and `ask_needs_place` to become true. This is a fail-open design in a safety-critical financial operation. When the remote state cannot be verified, the safe behavior is to suspend new order placement rather than assume that all previous orders have disappeared. ### Attack Path 1. The Skill has one or more GTC orders already live on the exchange. 2. The open-order API becomes unavailable, is rate-limited, returns malformed data, or is disrupted by an attacker capable of interfering with that service path. 3. `get_open_order_ids()` catches the exc ...[truncated 914 chars]- Remediation
View remediation
tuple[set[str], bool]: try: resp = get_client().get_open_orders() ids = { order["order_id"] for order in resp.get("orders", []) if order.get("order_id") } return ids, True except Exception as exc: print(f"[error] Could not verify open orders: {exc}") return set(), False ``` - Require the success flag before calling `run_market()` in live mode. - Reconcile local order IDs against authoritative exchange state before posting replacements. - Add idempotency or client-generated order identifiers if supported by the SDK. - Add bounded retries with exponential backoff for transient failures. - Emit a failure result to the automaton rather than reporting a successful run when reconciliation fails. ]]>