mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-06-06 23:51:39 +00:00
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.
This commit is contained in:
Adam Gessaman
@@ -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)`;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user