From 507e1385a6485a92853761db817c26f3bc4694d5 Mon Sep 17 00:00:00 2001
From: "alexei.dolgolyov" <dolgolyov.alexei@gmail.com>
Date: Sat, 23 May 2026 01:13:55 +0300
Subject: [PATCH] feat(ui/icon-select): defence-in-depth XSS sanitiser on icon
 channel
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Every IconSelect caller was audited: each builds item.icon from a
constant ICON_* literal, a lookup-table getter, or
renderDeviceIcon(stored_id) — none of which embed user input today.
The new sanitiseIcon() helper is the belt-and-braces guard for a
future caller that forgets the trusted-SVG contract: reject icon
strings containing <script>/<iframe>/<embed>/<object>/javascript:/on*=
markers, warn to the console, and fall back to empty so the cell still
renders the (escaped) label + desc.
---
 .../src/ledgrab/static/js/core/icon-select.ts | 50 +++++++++++++++++--
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/server/src/ledgrab/static/js/core/icon-select.ts b/server/src/ledgrab/static/js/core/icon-select.ts
index 2419b5c..6b67b20 100644
--- a/server/src/ledgrab/static/js/core/icon-select.ts
+++ b/server/src/ledgrab/static/js/core/icon-select.ts
@@ -44,6 +44,43 @@ function escAttr(text: string | undefined | null): string {
         .replace(/'/g, '&#39;');
 }
 
+/**
+ * Defensive sink for `IconSelectItem.icon`.
+ *
+ * `item.icon` is interpolated raw into the cell innerHTML (see `_buildGrid`
+ * and `_syncTrigger`). Every current caller (audited via the IconSelect
+ * caller scan) feeds the field with one of:
+ *   * a constant `ICON_*` literal,
+ *   * the output of a lookup table (`getValueSourceIcon`, `getColorStripIcon`,
+ *     `getDeviceTypeIcon`, …) — table values are project-owned constants,
+ *   * `renderDeviceIcon(d.icon)` — `d.icon` is a stored id mapped to a
+ *     project-owned SVG via `getDeviceIconDef`; unknown ids return `''`.
+ * None of those paths embed user input.
+ *
+ * The sanitiser here is **defence in depth** for a future caller who
+ * forgets that contract: reject icon strings containing `<script>` /
+ * `javascript:` / `on*=` attributes and fall back to the empty string
+ * so the cell still renders the label + desc. We deliberately do not
+ * try to "fix" the icon — silent partial sanitisation is worse than a
+ * missing icon for a developer who needs to spot the regression.
+ */
+const _DANGEROUS_ICON = /<script\b|<\/script>|\bjavascript:|\s+on[a-z]+\s*=|<iframe\b|<embed\b|<object\b/i;
+
+function sanitiseIcon(icon: string | undefined | null): string {
+    if (!icon) return '';
+    if (_DANGEROUS_ICON.test(icon)) {
+        // Loud about it in dev consoles; silent in production builds where
+        // `console.warn` is typically tree-shaken away.
+        try {
+            console.warn('[IconSelect] Rejected icon containing forbidden HTML markers');
+        } catch {
+            // window.console can be undefined in odd embed scenarios.
+        }
+        return '';
+    }
+    return icon;
+}
+
 /** All registered IconSelect instances; lets `closeAllIconSelects` reach scroll-listener state. */
 const _registry: Set<IconSelect> = new Set();
 
@@ -299,11 +336,13 @@ export class IconSelect {
         // item.icon is a raw SVG string by design (callers pass project-owned
         // icon literals). label/desc/value are user-visible text and may
         // originate from user input — escape them everywhere they cross
-        // an innerHTML boundary.
+        // an innerHTML boundary. `sanitiseIcon` is the defence-in-depth
+        // guard for icon strings that should never reach here from user
+        // input — see its block comment for the audit conclusion.
         const cells = this._items.map(item => {
             const search = (item.label + ' ' + (item.desc || '')).toLowerCase();
             return `<div class="icon-select-cell" data-value="${escAttr(item.value)}" data-search="${escAttr(search)}">
-                <span class="icon-select-cell-icon">${item.icon}</span>
+                <span class="icon-select-cell-icon">${sanitiseIcon(item.icon)}</span>
                 <span class="icon-select-cell-label">${escapeHtml(item.label)}</span>
                 ${item.desc ? `<span class="icon-select-cell-desc">${escapeHtml(item.desc)}</span>` : ''}
             </div>`;
@@ -320,7 +359,7 @@ export class IconSelect {
         const item = this._items.find(i => i.value === val);
         if (item) {
             this._trigger.innerHTML =
-                `<span class="icon-select-trigger-icon">${item.icon}</span>` +
+                `<span class="icon-select-trigger-icon">${sanitiseIcon(item.icon)}</span>` +
                 `<span class="icon-select-trigger-label">${escapeHtml(item.label)}</span>` +
                 `<span class="icon-select-trigger-arrow">&#x25BE;</span>`;
         } else if (this._placeholder) {
@@ -493,10 +532,11 @@ export function showTypePicker({ title, items, onPick, filterTabs, onFilterChang
     function buildCells(cellItems: IconSelectItem[]): string {
         // item.icon is trusted raw SVG. label/desc/value are escaped at
         // every innerHTML boundary because callers route user-typed text
-        // (device names, entity labels) through this picker.
+        // (device names, entity labels) through this picker. `sanitiseIcon`
+        // is defence-in-depth for the icon channel — see its block comment.
         return cellItems.map(item =>
             `<div class="icon-select-cell" data-value="${escAttr(item.value)}" data-search="${escAttr((item.label + ' ' + (item.desc || '')).toLowerCase())}" role="option">
-                <span class="icon-select-cell-icon">${item.icon}</span>
+                <span class="icon-select-cell-icon">${sanitiseIcon(item.icon)}</span>
                 <span class="icon-select-cell-label">${escapeHtml(item.label)}</span>
                 ${item.desc ? `<span class="icon-select-cell-desc">${escapeHtml(item.desc)}</span>` : ''}
             </div>`