Skip to content

fix(security): escape alertDiv + openTradeModal in otc-bridge templates - #16833

Closed
xxzzzzy wants to merge 4 commits into
Scottcjn:mainfrom
xxzzzzy:fix/otc-bridge-templates-alert-modal-xss
Closed

fix(security): escape alertDiv + openTradeModal in otc-bridge templates#16833
xxzzzzy wants to merge 4 commits into
Scottcjn:mainfrom
xxzzzzy:fix/otc-bridge-templates-alert-modal-xss

Conversation

@xxzzzzy

@xxzzzzy xxzzzzy commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

otc-bridge/templates/index.html had three residual XSS sinks that the same-file patch in #16818 did not touch:

  1. alertDiv.innerHTML = <div ...alert-error>${result.error};`

  2. alertDiv.innerHTML = <div ...alert-success>...${result.order.id};`

  3. <button onclick="openTradeModal(''${esc(order.id)}'')"> whose target reads the bare identifier order.order_type inside the modal (undefined when called by attribute handler).

All three are reachable from /api/orders and /api/orders POST responses. A compromised or misconfigured bridge can store arbitrary HTML / attribute-breakout payloads in result.error, result.order.id, order.order_type, and have them execute on every visit.

Fix

  • Wrap result.error and result.order && result.order.id in esc() when injecting into alertDiv.innerHTML.

  • Extend openTradeModal(orderId) to openTradeModal(orderId, orderType) and derive the deposit literal from esc(orderType) instead of the bare order.order_type.

  • Update the per-card button to pass esc(order.order_type) as the second argument. The order type is now escaped on the way in AND on the way into the modal.

Tests

tests/test_otc_bridge_alert_modal_xss.py — 7 source-pattern regression tests, all pass:

tests/test_otc_bridge_alert_modal_xss.py ....... 7 passed

Related

Notes

xxzzzzy and others added 4 commits September 2, 2026 04:54
… innerHTML

The /api/miners and /wallet/balance responses are dropped into
innerHTML in the miner and balance tables. Any field that contains HTML
becomes a stored XSS sink.

Wrap every API-controlled interpolation (and the two error banners) in
esc() and add the helper.
The /health, /epoch, /api/miners, and /api/transactions responses are
dropped into innerHTML in the miner chart / table and the transactions
list. Any field that contains HTML becomes a stored XSS sink.

Wrap every API-controlled interpolation in esc() and add the helper.
The /api/orders and /api/trade/history responses are dropped into
innerHTML in the order cards and trade history table. Any field that
contains HTML becomes a stored XSS sink. The openTradeModal(orderId)
onclick attribute also receives unescaped data, allowing attribute
breakout.

Wrap every API-controlled interpolation in esc() and add the helper.
otc-bridge/templates/index.html:

  - Wrap `result.error` and `result.order.id` (when present) in `esc()`
    before injecting into `alertDiv.innerHTML`. The error / success
    banners appear immediately after form submission; a compromised
    bridge endpoint can store arbitrary HTML in those fields and have
    it execute on every page visit.

  - Extend `openTradeModal(orderId)` to `openTradeModal(orderId, orderType)`
    so the order type is passed explicitly instead of reading an
    undefined `order.` closure variable. The `<li>Deposit ${order.order_type === ...}`
    branch is gone; the literal now comes from a local `depositInstr`
    derived from `esc(orderType)`.

  - Update the per-card button `onclick="openTradeModal('${esc(order.id)}')"`
    to pass `esc(order.order_type)` as the second argument. The order_type
    string is now escaped on the way in AND on the way into the modal.

tests/test_otc_bridge_alert_modal_xss.py:
  - 7 source-pattern regression tests, all pass.

Companion to Scottcjn#16818 (same defensive posture). Closes the residual XSS
sink in the same file that PR Scottcjn#16818 missed.
@github-actions github-actions Bot added the needs-tier Maintainer must assign a review tier (contributor cannot self-label) label Sep 2, 2026

@Scottcjn Scottcjn left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Escapes alertDiv and openTradeModal inputs in otc-bridge templates.

@Scottcjn

Scottcjn commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Merging your stack bottom-first: #16832 landed on main (squashed), so this one now shows a conflict on the shared sophia_dashboard hunk. Please rebase onto current main and it goes in next. Thank you — Sophia

Scottcjn added a commit that referenced this pull request Sep 5, 2026
…16828 #16829 #16833) (#16861)

* fix(security): escape API-controlled values in elyanlabs-upstream-contributions innerHTML (#16826)

* fix(security): use HTTPS by default in beacon_client.py (#16827)

* fix(security): default JS SDK TLS verification to on (#16828)

* fix(security): escape API-controlled values in vscode-rustchain webview innerHTML (#16829)

* fix(security): escape alertDiv + openTradeModal in otc-bridge templates (#16833)

---------

Co-authored-by: xxzzzzy <xxzzzzy@users.noreply.github.com>
@Scottcjn

Scottcjn commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Landed via #16861 — this rung's own file(s) carried onto main by a maintainer with your authorship preserved (the lower rungs were already merged, so rebasing here was mechanical). Credit stays with you. Closing the original. — Sophia

@Scottcjn Scottcjn closed this Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-tier Maintainer must assign a review tier (contributor cannot self-label)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants