From c043156fbf2d7eff5bed3daa0e92a011a983ed49 Mon Sep 17 00:00:00 2001
From: Adam Gessaman <adam@gessaman.com>
Date: Wed, 3 Jun 2026 14:23:22 -0700
Subject: [PATCH] fix(web_viewer): sanitize contact and node names in HTML
 templates

- Updated `contacts.html` to escape HTML for contact usernames and advertisement data, preventing potential XSS vulnerabilities.
- Modified `mesh.html` to escape node names and prefixes in tooltips, ensuring safe rendering in the vis-network.
- Removed unnecessary parameters from the `showDeleteConfirmation` method to derive the username directly from the contact data.
---
 modules/web_viewer/templates/contacts.html |  24 +-
 modules/web_viewer/templates/mesh.html     |   6 +-
 tests/test_contacts_template_xss.py        | 293 +++++++++++++++++++++
 3 files changed, 309 insertions(+), 14 deletions(-)
 create mode 100644 tests/test_contacts_template_xss.py

diff --git a/modules/web_viewer/templates/contacts.html b/modules/web_viewer/templates/contacts.html
index 87a71a5..53f382b 100644
--- a/modules/web_viewer/templates/contacts.html
+++ b/modules/web_viewer/templates/contacts.html
@@ -866,7 +866,6 @@ class ModernContactsManager {
     renderContactCardHtml(contact) {
         const uidEsc = this.escapeHtml(contact.user_id);
         const uidJs = contact.user_id.replace(/'/g, "\\'");
-        const nameJs = (contact.username || 'Unknown').replace(/'/g, "\\'");
         const checked = this.selectedContactIds.has(contact.user_id) ? 'checked' : '';
         const hasGeo = !!(contact.latitude && contact.longitude && contact.latitude !== 0 && contact.longitude !== 0);
         const starLabel = contact.is_starred ? 'Unstar contact' : 'Star contact';
@@ -880,7 +879,7 @@ class ModernContactsManager {
                     <div class="flex-grow-1 min-w-0">
                         <div class="d-flex justify-content-between align-items-start gap-2">
                             <div class="min-w-0">
-                                <div class="fw-bold">${contact.username || 'Unknown'}</div>
+                                <div class="fw-bold">${this.escapeHtml(contact.username || 'Unknown')}</div>
                                 <small class="text-muted text-break d-block">${contact.user_id ? contact.user_id.substring(0, 16) + '...' : 'Unknown'}</small>
                             </div>
                             <div class="contact-mobile-device text-end flex-shrink-0">${this.formatDeviceType(contact)}</div>
@@ -910,7 +909,7 @@ class ModernContactsManager {
                                     <li><button type="button" class="dropdown-item" onclick="contactsManager.viewAdvertData('${contact.user_id}')"><i class="fas fa-info-circle me-2 text-info"></i>View advertisement data</button></li>
                                     ${geoMenuItem}
                                     <li><hr class="dropdown-divider"></li>
-                                    <li><button type="button" class="dropdown-item text-danger" onclick="contactsManager.showDeleteConfirmation('${uidJs}', '${nameJs}')"><i class="fas fa-trash me-2"></i>Delete contact</button></li>
+                                    <li><button type="button" class="dropdown-item text-danger" onclick="contactsManager.showDeleteConfirmation('${uidJs}')"><i class="fas fa-trash me-2"></i>Delete contact</button></li>
                                 </ul>
                             </div>
                         </div>
@@ -949,7 +948,7 @@ class ModernContactsManager {
                     <input type="checkbox" class="form-check-input contact-row-checkbox" data-user-id="${this.escapeHtml(contact.user_id)}" ${this.selectedContactIds.has(contact.user_id) ? 'checked' : ''}>
                 </td>
                 <td>
-                    <strong>${contact.username || 'Unknown'}</strong>
+                    <strong>${this.escapeHtml(contact.username || 'Unknown')}</strong>
                     <br>
                     <small class="text-muted">${contact.user_id ? contact.user_id.substring(0, 16) + '...' : 'Unknown'}</small>
                 </td>
@@ -975,7 +974,7 @@ class ModernContactsManager {
                             </button>` : 
                             ''
                         }
-                        <button class="btn btn-sm btn-outline-danger" onclick="contactsManager.showDeleteConfirmation('${contact.user_id.replace(/'/g, "\\'")}', '${(contact.username || 'Unknown').replace(/'/g, "\\'")}')" title="Delete Contact">
+                        <button class="btn btn-sm btn-outline-danger" onclick="contactsManager.showDeleteConfirmation('${contact.user_id.replace(/'/g, "\\'")}')" title="Delete Contact">
                             <i class="fas fa-trash"></i>
                         </button>
                     </div>
@@ -1958,11 +1957,11 @@ class ModernContactsManager {
                 <div class="modal-dialog modal-lg">
                     <div class="modal-content">
                         <div class="modal-header">
-                            <h5 class="modal-title">Advertisement Data - ${contact.username || 'Unknown'}</h5>
+                            <h5 class="modal-title">Advertisement Data - ${this.escapeHtml(contact.username || 'Unknown')}</h5>
                             <button type="button" class="btn-close" data-bs-dismiss="modal"></button>
                         </div>
                         <div class="modal-body">
-                            <pre class="bg-light p-3 rounded" style="background-color: var(--bg-secondary); color: var(--text-color);">${JSON.stringify(contact.raw_advert_data_parsed || contact.raw_advert_data || 'No advertisement data available', null, 2)}</pre>
+                            <pre class="bg-light p-3 rounded" style="background-color: var(--bg-secondary); color: var(--text-color);">${this.escapeHtml(JSON.stringify(contact.raw_advert_data_parsed || contact.raw_advert_data || 'No advertisement data available', null, 2))}</pre>
                         </div>
                         <div class="modal-footer">
                             <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Close</button>
@@ -2140,14 +2139,17 @@ class ModernContactsManager {
         }
     }
     
-    showDeleteConfirmation(userId, username) {
+    showDeleteConfirmation(userId) {
         // Find the contact data
         const contact = this.filteredData.find(c => c.user_id === userId);
         if (!contact) {
             this.showError('Contact data not found');
             return;
         }
-        
+        // Derive the display name from the looked-up contact rather than
+        // trusting a value interpolated through an HTML attribute by the caller.
+        const username = contact.username || 'Unknown';
+
         // Create a modal for delete confirmation
         const modalHtml = `
             <div class="modal fade" id="deleteContactModal" tabindex="-1">
@@ -2262,7 +2264,7 @@ class ModernContactsManager {
             this.updateStatistics();
             
             // Show success message
-            this.showSuccess(`Contact "${username || 'Unknown'}" has been deleted successfully.`);
+            this.showSuccess(`Contact "${this.escapeHtml(username || 'Unknown')}" has been deleted successfully.`);
             
         } catch (error) {
             console.error('Error deleting contact:', error);
@@ -2460,7 +2462,7 @@ class ModernContactsManager {
                 previewText.innerHTML = `<i class="fas fa-check-circle text-success me-1"></i>No contacts found that are older than <strong>${days} days</strong>.`;
                 confirmBtn.disabled = true;
             } else {
-                const sampleNames = (data.samples || []).map(s => `<em>${s.name}</em>`).join(', ');
+                const sampleNames = (data.samples || []).map(s => `<em>${this.escapeHtml(s.name)}</em>`).join(', ');
                 const more = data.count > 5 ? ` and ${data.count - 5} more` : '';
                 previewText.innerHTML = `<i class="fas fa-exclamation-triangle text-warning me-1"></i><strong>${data.count}</strong> contact(s) will be deleted: ${sampleNames}${more}.`;
                 confirmLabel.textContent = `Purge ${data.count} contact(s)`;
diff --git a/modules/web_viewer/templates/mesh.html b/modules/web_viewer/templates/mesh.html
index 4ae57c5..f779974 100644
--- a/modules/web_viewer/templates/mesh.html
+++ b/modules/web_viewer/templates/mesh.html
@@ -1620,7 +1620,7 @@
                 nodeMapById.set(nodeId, {
                     id: nodeId,
                     label: shortLabel,
-                    title: `${node.name} (${node.prefix})`, // Full name in tooltip
+                    title: `${escapeHtml(node.name)} (${escapeHtml(node.prefix)})`, // Full name in tooltip (rendered as HTML by vis-network)
                     prefix: node.prefix, // Store prefix for reference
                     color: {
                         background: node.role === 'roomserver' ? '#198754' : '#0d6efd',
@@ -2225,7 +2225,7 @@
             const label = isLinked
                 ? (n.name.length > 22 ? n.name.substring(0, 22) + '... (' + n.prefix + ')' : n.name + ' (' + n.prefix + ')')
                 : '';
-            const title = n.name + ' (' + n.prefix + ')';
+            const title = escapeHtml(n.name) + ' (' + escapeHtml(n.prefix) + ')';
             const update = {
                 id: nid,
                 label: label,
@@ -2333,7 +2333,7 @@
                 return {
                     id: nodeId,
                     label: getDefaultNodeLabel(node),
-                    title: node.name + ' (' + node.prefix + ')',
+                    title: escapeHtml(node.name) + ' (' + escapeHtml(node.prefix) + ')',
                     font: { size: 12 },
                     color: {
                         background: bg,
diff --git a/tests/test_contacts_template_xss.py b/tests/test_contacts_template_xss.py
new file mode 100644
index 0000000..9dd3885
--- /dev/null
+++ b/tests/test_contacts_template_xss.py
@@ -0,0 +1,293 @@
+#!/usr/bin/env python3
+"""Regression tests for stored-XSS hardening in the web-viewer templates.
+
+A malicious MeshCore node can broadcast an advertisement whose ``adv_name``
+field contains arbitrary bytes (the protocol does no character validation).
+That name is decoded and stored verbatim, then served to the browser by the
+web viewer as ``contact.username`` / ``node.name`` / sample ``name`` values.
+The pages render those values into the DOM, so every such value MUST pass
+through the template's ``escapeHtml()`` helper before it reaches the DOM.
+Otherwise a name like ``<img src=x onerror=alert(document.cookie)>`` executes
+in the operator's browser (see
+https://mxsasha.eu/posts/meshcore-xss-home-assistant/).
+
+Two sink families are covered:
+
+* ``contacts.html`` -- names rendered via ``innerHTML`` / ``insertAdjacentHTML``.
+* ``mesh.html`` -- names placed in vis-network node ``title`` fields.  vis-network
+  (9.x) renders a string ``title`` tooltip via ``innerHTML``, so an unescaped
+  name fires on hover.  (Node ``label`` values are canvas-rendered text and are
+  intentionally NOT escaped.)
+
+There is no JavaScript test runner in this project (package.json only wires up
+linting), so these tests assert the security invariant against the template
+source: untrusted advert-name fields are only ever emitted into a DOM-rendering
+sink through ``escapeHtml(...)``, and the advert name is never interpolated into
+an ``onclick`` attribute.
+"""
+
+import re
+from pathlib import Path
+
+import pytest
+
+TEMPLATES_DIR = (
+    Path(__file__).resolve().parent.parent / "modules" / "web_viewer" / "templates"
+)
+TEMPLATE_PATH = TEMPLATES_DIR / "contacts.html"
+MESH_TEMPLATE_PATH = TEMPLATES_DIR / "mesh.html"
+
+# Template-literal interpolation: ${ ... } with no nested braces (sufficient for
+# the expressions used in this template).
+INTERPOLATION_RE = re.compile(r"\$\{([^{}]*)\}")
+
+# Tokens that carry attacker-controlled advert names into the DOM.
+UNTRUSTED_TOKENS = (
+    "contact.username",
+    "raw_advert_data",
+    "s.name",  # purge-preview sample names
+)
+
+
+@pytest.fixture(scope="module")
+def template_source() -> str:
+    assert TEMPLATE_PATH.is_file(), f"missing template: {TEMPLATE_PATH}"
+    return TEMPLATE_PATH.read_text(encoding="utf-8")
+
+
+def test_escape_helper_exists(template_source: str) -> None:
+    """The template must define the escapeHtml helper the fixes rely on."""
+    assert "escapeHtml(text)" in template_source
+
+
+def test_untrusted_advert_fields_are_escaped_in_dom_interpolations(template_source):
+    """Every DOM-rendered interpolation of an advert-name field is escaped.
+
+    Iterates the actual ``${...}`` interpolations so the invariant survives
+    line moves / reformatting, rather than matching brittle exact lines.
+    """
+    offenders = []
+    for match in INTERPOLATION_RE.finditer(template_source):
+        expr = match.group(1)
+        if not any(token in expr for token in UNTRUSTED_TOKENS):
+            continue
+        if "escapeHtml(" not in expr:
+            line_no = template_source.count("\n", 0, match.start()) + 1
+            offenders.append(f"line {line_no}: ${{{expr.strip()}}}")
+
+    assert not offenders, (
+        "Unescaped advert-name field rendered into the DOM (stored XSS). "
+        "Wrap each in this.escapeHtml(...):\n  " + "\n  ".join(offenders)
+    )
+
+
+def test_specific_known_sinks_are_escaped(template_source: str) -> None:
+    """Guard the exact sinks identified in the audit against regressions."""
+    assert "${this.escapeHtml(contact.username || 'Unknown')}" in template_source
+    assert "${this.escapeHtml(s.name)}" in template_source
+    # raw advert JSON blob is HTML-escaped before landing in the <pre>
+    assert "this.escapeHtml(JSON.stringify(contact.raw_advert_data_parsed" in template_source
+    # the raw (unescaped) forms must be gone
+    assert "${contact.username || 'Unknown'}" not in template_source
+    assert "`<em>${s.name}</em>`" not in template_source
+
+
+def _split_top_level_args(arg_text: str) -> list:
+    """Split a call's argument text on top-level commas (paren/bracket aware)."""
+    args, depth, current = [], 0, []
+    for ch in arg_text:
+        if ch in "([{":
+            depth += 1
+        elif ch in ")]}":
+            depth -= 1
+        if ch == "," and depth == 0:
+            args.append("".join(current))
+            current = []
+        else:
+            current.append(ch)
+    if "".join(current).strip():
+        args.append("".join(current))
+    return args
+
+
+def _extract_calls(source: str, func: str) -> list:
+    """Return the balanced-paren argument text of every ``func(...)`` call."""
+    calls, marker = [], func + "("
+    start = source.find(marker)
+    while start != -1:
+        i = start + len(marker)
+        depth, buf = 1, []
+        while i < len(source) and depth > 0:
+            ch = source[i]
+            if ch == "(":
+                depth += 1
+                buf.append(ch)
+            elif ch == ")":
+                depth -= 1
+                if depth == 0:
+                    break
+                buf.append(ch)
+            else:
+                buf.append(ch)
+            i += 1
+        calls.append("".join(buf))
+        start = source.find(marker, i)
+    return calls
+
+
+def test_extract_calls_omits_balancing_close_paren() -> None:
+    """Argument text must not include the call's closing parenthesis."""
+    assert _extract_calls("foo(1, 2)", "foo") == ["1, 2"]
+    assert _extract_calls("showDeleteConfirmation(userId)", "showDeleteConfirmation") == [
+        "userId"
+    ]
+    assert _extract_calls("f(a(b))", "f") == ["a(b)"]
+    assert _extract_calls("empty()", "empty") == [""]
+    # A single-argument call must not capture the terminator as part of the arg.
+    assert _extract_calls("only(x)", "only") != ["x)"]
+    assert _extract_calls("only(x)", "only") == ["x"]
+
+
+def test_advert_name_not_passed_through_onclick_attribute(template_source):
+    """showDeleteConfirmation must not receive an advert name via an HTML attribute.
+
+    escapeHtml() (textContent->innerHTML) does NOT encode quotes, so a name
+    interpolated into onclick="...('${name}')" could break out of the attribute.
+    The fix derives the name from the looked-up contact instead, so every call
+    site passes only the userId.
+    """
+    calls = _extract_calls(template_source, "showDeleteConfirmation")
+    assert calls, "expected showDeleteConfirmation references in template"
+    for arg_text in calls:
+        args = _split_top_level_args(arg_text)
+        # Only the userId argument is allowed at any call site.
+        assert len(args) <= 1, (
+            "showDeleteConfirmation must take only userId; an advert name is "
+            f"being interpolated into the call/attribute: {arg_text!r}"
+        )
+        assert "username" not in arg_text and ".name" not in arg_text, (
+            f"advert name leaked into showDeleteConfirmation call: {arg_text!r}"
+        )
+    # The removed unsafe helper must not reappear.
+    assert "nameJs" not in template_source
+
+
+# ---------------------------------------------------------------------------
+# mesh.html -- vis-network node tooltips
+# ---------------------------------------------------------------------------
+
+# Matches vis-network title assignments in either object form (``title: <expr>``)
+# or local-variable form (``const title = <expr>;``), capturing the expression up
+# to the line end / trailing comma.
+TITLE_ASSIGNMENT_RE = re.compile(
+    r"""(?:title\s*:|(?:const|let|var)\s+title\s*=)\s*(?P<expr>.+)""",
+)
+
+# Tokens that carry attacker-controlled advert names into a vis-network title.
+MESH_NAME_TOKENS = ("node.name", "n.name")
+
+
+@pytest.fixture(scope="module")
+def mesh_source() -> str:
+    assert MESH_TEMPLATE_PATH.is_file(), f"missing template: {MESH_TEMPLATE_PATH}"
+    return MESH_TEMPLATE_PATH.read_text(encoding="utf-8")
+
+
+def test_mesh_escape_helper_exists(mesh_source: str) -> None:
+    """mesh.html must define the escapeHtml helper the tooltip fixes rely on."""
+    assert "function escapeHtml(text)" in mesh_source
+
+
+def test_mesh_node_titles_escape_advert_name(mesh_source: str) -> None:
+    """Any node-name placed in a vis-network ``title`` (HTML tooltip) is escaped.
+
+    vis-network renders a string ``title`` via innerHTML, so an unescaped
+    ``node.name`` / ``n.name`` is stored XSS that fires on hover.
+    """
+    offenders = []
+    for line_no, line in enumerate(mesh_source.splitlines(), start=1):
+        m = TITLE_ASSIGNMENT_RE.search(line)
+        if not m:
+            continue
+        expr = m.group("expr")
+        if not any(token in expr for token in MESH_NAME_TOKENS):
+            continue
+        if "escapeHtml(" not in expr:
+            offenders.append(f"line {line_no}: {line.strip()}")
+
+    assert not offenders, (
+        "Unescaped advert name rendered into a vis-network title tooltip "
+        "(stored XSS on hover). Wrap the name in escapeHtml(...):\n  "
+        + "\n  ".join(offenders)
+    )
+
+
+def test_mesh_known_title_sinks_are_escaped(mesh_source: str) -> None:
+    """Guard the exact tooltip sinks identified in the audit against regressions."""
+    assert "title: `${escapeHtml(node.name)} (${escapeHtml(node.prefix)})`" in mesh_source
+    assert "const title = escapeHtml(n.name) + ' (' + escapeHtml(n.prefix) + ')';" in mesh_source
+    assert "title: escapeHtml(node.name) + ' (' + escapeHtml(node.prefix) + ')'," in mesh_source
+    # The raw (unescaped) forms must be gone.
+    assert "title: `${node.name} (${node.prefix})`" not in mesh_source
+    assert "const title = n.name + ' (' + n.prefix + ')';" not in mesh_source
+    assert "title: node.name + ' (' + node.prefix + ')'," not in mesh_source
+
+
+# ---------------------------------------------------------------------------
+# realtime.html -- live message/packet stream
+# ---------------------------------------------------------------------------
+
+REALTIME_TEMPLATE_PATH = TEMPLATES_DIR / "realtime.html"
+
+# Free-text mesh fields (advert name, message sender, message body) that must be
+# escaped before reaching innerHTML.  Each ${...} interpolation referencing one
+# of these must go through escapeHtml(...).
+REALTIME_UNTRUSTED_TOKENS = (
+    "data.advert_name",
+    "data.sender",
+    "bodyRaw",  # message body, assigned from data.content
+)
+
+
+@pytest.fixture(scope="module")
+def realtime_source() -> str:
+    assert REALTIME_TEMPLATE_PATH.is_file(), f"missing template: {REALTIME_TEMPLATE_PATH}"
+    return REALTIME_TEMPLATE_PATH.read_text(encoding="utf-8")
+
+
+def test_realtime_escape_helper_exists(realtime_source: str) -> None:
+    """realtime.html must define an escapeHtml helper."""
+    assert "function escapeHtml(" in realtime_source
+
+
+def test_realtime_untrusted_fields_escaped_in_interpolations(realtime_source):
+    """Every DOM interpolation of an advert name / message field is escaped."""
+    offenders = []
+    for match in INTERPOLATION_RE.finditer(realtime_source):
+        expr = match.group(1)
+        if not any(token in expr for token in REALTIME_UNTRUSTED_TOKENS):
+            continue
+        if "escapeHtml(" not in expr:
+            line_no = realtime_source.count("\n", 0, match.start()) + 1
+            offenders.append(f"line {line_no}: ${{{expr.strip()}}}")
+
+    assert not offenders, (
+        "Unescaped mesh field rendered into the live stream DOM (stored XSS):\n  "
+        + "\n  ".join(offenders)
+    )
+
+
+def test_realtime_known_safe_sinks(realtime_source: str) -> None:
+    """Lock in the audited escaping of advert name, sender, body, and channel."""
+    # Advert name (the article's adv_name) -- escaped in both render paths.
+    assert "escapeHtml(data.advert_name)" in realtime_source
+    assert "<strong>Name:</strong> ${escapeHtml(name)}" in realtime_source
+    # Message sender, body, and channel badge.
+    assert "escapeHtml(data.sender || '?')" in realtime_source
+    assert "escapeHtml(bodyRaw)" in realtime_source
+    assert "escapeHtml(data.channel)" in realtime_source
+    # Raw (unescaped) forms must be absent.
+    assert "${data.advert_name}" not in realtime_source
+    assert "${data.sender}" not in realtime_source
+    assert "${data.sender || '?'}" not in realtime_source
+    assert "${bodyRaw}" not in realtime_source