From 57395e2b28bb6edeb69557fc8d318332b9598ea5 Mon Sep 17 00:00:00 2001 From: nullrouten0 Date: Thu, 8 Oct 2026 13:59:25 -0700 Subject: [PATCH] fix(ui): escape observer iata, name and id, and node names, in eight render sinks (#2118) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Anyone who can publish to an MQTT broker that CoreScope reads controls three strings: the observer `iata` (a topic segment), the observer `name` (the JSON `origin` field) and the observer `id` (a topic segment). Anyone with a radio controls the node `name` in an ADVERT. Eight places wrote these into the page with `innerHTML` without escaping, so a crafted value runs as script in every visitor's browser: | file | what | |---|---| | `packets.js` | Region badge in the group, child and single packet rows | | `observers.js` | Region badge in the observers table and in the slide-over | | `live.js` | "Heard By โ€” Regions" header in the node detail | | `nodes.js` | observer name in the clock-skew evidence panel | | `analytics.js` | `observer_id` inside `data-observer=` and `id=` attributes | | `app.js` | node name in the favorites dropdown | | `customize-v2.js` | node name in the geofilter prune preview (admin page) | **Fix:** each value now goes through the existing `escapeHtml()` / `esc()` helper. No behaviour change for normal values. **Tests:** one test per sink added to `tests/unit/test-xss-escape-sinks.js`, following the file's existing pattern (pull the template out of the source, render it with a hostile value, check the output is escaped). Reverting any single fix makes its test fail. 41/41 pass. **Notes for reviewers:** - The ingestor uppercases `iata` before storing it. That is not a defence: tag and attribute names are case-insensitive, and script can be written with numeric character references. - An operator who has set an `observerIATAWhitelist` is protected from the `iata` sinks but not from the name/id sinks. - Follow-ups worth a separate PR: validate `iata` and observer `id` to a safe character set at ingest, cap the `origin` length, and add a `Content-Security-Policy` header. This PR is escaping only so it is easy to review and safe to ship. Running in production on our instance since 2026-10-07. ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Mythos 5.1 --- public/analytics.js | 4 +- public/app.js | 2 +- public/customize-v2.js | 2 +- public/live.js | 2 +- public/nodes.js | 2 +- public/observers.js | 4 +- public/packets.js | 6 +- tests/unit/test-xss-escape-sinks.js | 95 +++++++++++++++++++++++++++++ 8 files changed, 106 insertions(+), 11 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index c39d1818..7851bdb2 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -4319,13 +4319,13 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _stopDistanceRet else if (obs.current_noise_floor >= -100) nfClass = 'rf-nf-warning'; } - return `
+ return `
${esc(name)} ${nf} dBm ${batt ? `${batt}` : ''}
-
+
avg: ${avgNf} max: ${maxNf} diff --git a/public/app.js b/public/app.js index e5677a3f..7a04414f 100644 --- a/public/app.js +++ b/public/app.js @@ -1734,7 +1734,7 @@ window.addEventListener('DOMContentLoaded', () => { const statusLabel = age === null ? 'unknown' : age < HEALTH_THRESHOLDS.nodeDegradedMs ? 'healthy' : age < HEALTH_THRESHOLDS.nodeSilentMs ? 'degraded' : 'silent'; return '' + '' - + '' + (h.node.name || truncate(pk, 12)) + '' + + '' + escapeHtml(h.node.name || truncate(pk, 12)) + '' + '' + (h.stats.lastHeard ? timeAgo(h.stats.lastHeard) : 'never') + '' + favStar(pk, 'fav-dd-star') + ''; diff --git a/public/customize-v2.js b/public/customize-v2.js index d9ae07cf..2a01888b 100644 --- a/public/customize-v2.js +++ b/public/customize-v2.js @@ -2069,7 +2069,7 @@ } listEl.innerHTML = _gfPruneNodes.map(function (n) { var coords = n.lat != null ? (' ยท ' + n.lat.toFixed(4) + ', ' + n.lon.toFixed(4)) : ''; - return '
' + (n.name || n.pubkey.slice(0, 12)) + coords + '
'; + return '
' + esc(n.name || n.pubkey.slice(0, 12)) + coords + '
'; }).join(''); confirmBtn.textContent = 'Delete ' + count + ' node' + (count !== 1 ? 's' : ''); resultEl.style.display = ''; diff --git a/public/live.js b/public/live.js index 1f0870c2..abee9316 100644 --- a/public/live.js +++ b/public/live.js @@ -2644,7 +2644,7 @@ if (observers.length) { const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))]; - html += `

Heard By${regions.length ? ' โ€” Regions: ' + regions.join(', ') : ''}

+ html += `

Heard By${regions.length ? ' โ€” Regions: ' + regions.map(r => escapeHtml(r)).join(', ') : ''}

` + observers.map(o => ``).join('') + '
'; diff --git a/public/nodes.js b/public/nodes.js index 7be18556..ce5e13b5 100644 --- a/public/nodes.js +++ b/public/nodes.js @@ -1205,7 +1205,7 @@ } var header = '
Hash ' + shortHash + ' ยท ' + obsCount + ' observer' + (obsCount !== 1 ? 's' : '') + ' ยท median corrected: ' + medianLabel + '
'; var lines = (ev.observers || []).map(function(o) { - var name = o.observerName || o.observerID; + var name = escapeHtml(o.observerName || o.observerID); return '
' + name + ' raw=' + formatSkew(o.rawSkewSec) + ' corrected=' + formatSkew(o.correctedSkewSec) + ' (observer offset ' + formatSkew(o.observerOffsetSec) + ')' + '
'; diff --git a/public/observers.js b/public/observers.js index 9a58c978..90f3c607 100644 --- a/public/observers.js +++ b/public/observers.js @@ -413,7 +413,7 @@ window.preserveCompareSelection = function preserveCompareSelection(prevIds, tbo return ` ${shapeIcon} ${h.label} ${escapeHtml(o.name || o.id)}${window.ObserversNaiveChip.render(o)}${o.can_relay === false ? ' listener' : (o.can_relay === true ? ' repeater' : '')} - ${o.iata ? `${o.iata}` : 'โ€”'} + ${o.iata ? `${escapeHtml(o.iata)}` : 'โ€”'} ${timeAgo(o.last_seen)} ${o.last_packet_at ? timeAgo(o.last_packet_at) : 'โ€”'} ${packetBadge(o)} @@ -526,7 +526,7 @@ window.preserveCompareSelection = function preserveCompareSelection(prevIds, tbo (naiveChipHTML ? '
' + naiveChipHTML + ' Clock is naive โ€” per-packet timing clamped to ingest time. Clears itself 24h after the last skew event.
' : '') + '
' + '
Status
' + h.label + '
' + - '
Region
' + (o.iata ? '' + o.iata + '' : 'โ€”') + '
' + + '
Region
' + (o.iata ? '' + escapeHtml(o.iata) + '' : 'โ€”') + '
' + '
Last status
' + timeAgo(o.last_seen) + '
' + '
Last packet
' + (o.last_packet_at ? timeAgo(o.last_packet_at) : 'โ€”') + '
' + '
Total packets
' + (o.packet_count || 0).toLocaleString() + '
' + diff --git a/public/packets.js b/public/packets.js index eb60ab6a..fc51b8f7 100644 --- a/public/packets.js +++ b/public/packets.js @@ -2546,7 +2546,7 @@ const _grpStyle = _grpHashStripe + _grpChanStyle; let html = ` ${isSingle ? '' : (isExpanded ? '' : '')} - ${groupRegion ? `${groupRegion}` : 'โ€”'} + ${groupRegion ? `${escapeHtml(groupRegion)}` : 'โ€”'} ${renderTimestampCell(p.latest)} ${truncate(p.hash || 'โ€”', 8)} ${groupSize ? groupSize + 'B' : 'โ€”'} @@ -2581,7 +2581,7 @@ const childPathStr = renderPath(childPath, c.observer_id, { summary: true }); const _childHashStripe = _hashStripeStyle(c.hash || p.hash); html += ` - ${childRegion ? `${childRegion}` : 'โ€”'} + ${childRegion ? `${escapeHtml(childRegion)}` : 'โ€”'} ${renderTimestampCell(c.timestamp)} ${truncate(c.hash || '', 8)} ${size}B @@ -2618,7 +2618,7 @@ const _flatHashStripe = _hashStripeStyle(p.hash); const _flatStyle = _flatHashStripe + _chanStyle; return ` - ${region ? `${region}` : 'โ€”'} + ${region ? `${escapeHtml(region)}` : 'โ€”'} ${renderTimestampCell(p.timestamp)} ${truncate(p.hash || String(p.id), 8)} ${size}B diff --git a/tests/unit/test-xss-escape-sinks.js b/tests/unit/test-xss-escape-sinks.js index 5867f9ee..2e476781 100644 --- a/tests/unit/test-xss-escape-sinks.js +++ b/tests/unit/test-xss-escape-sinks.js @@ -600,6 +600,101 @@ test('app.js unknown-route page escapes the route taken from location.hash', () assertNoXss(html, 'app.js unknown route'); }); +// ========================================================================= +// H. Observer-controlled strings (iata / name / id) and node names +// These values come straight from the MQTT topic or JSON (observer iata, +// name, id) or from an ADVERT packet (node name). Anyone who can publish +// to a broker, or transmit on the mesh, controls them. +// ========================================================================= +console.log('\n=== H. observer iata / name / id and node-name sinks ==='); + +// Like evalTemplate, but for a string-concatenation expression rather than +// a template literal. The captured group is evaluated as a JS expression. +function evalConcat(srcFile, regex, bindings) { + const src = fs.readFileSync(srcFile, 'utf8'); + const m = src.match(regex); + if (!m) throw new Error(`expression not found in ${srcFile} via ${regex}`); + const argNames = Object.keys(bindings); + const args = Object.values(bindings); + const truncate = (s, n) => (s == null ? '' : String(s).slice(0, n)); + const fn = new Function(...argNames, 'escapeHtml', 'esc', 'truncate', 'return ' + m[1] + ';'); + return fn(...args, escapeHtml, escapeHtml, truncate); +} + +const PAYLOAD = TAG_PAYLOAD + ATTR_PAYLOAD; + +test('packets.js group row: Region badge escapes observer iata', () => { + const html = evalTemplate('public/packets.js', + /(\$\{groupRegion[\s\S]*?<\/td>)/, { groupRegion: PAYLOAD }); + assertNoXss(html, 'packets.js groupRegion'); +}); + +test('packets.js child row: Region badge escapes observer iata', () => { + const html = evalTemplate('public/packets.js', + /(\$\{childRegion[\s\S]*?<\/td>)/, { childRegion: PAYLOAD }); + assertNoXss(html, 'packets.js childRegion'); +}); + +test('packets.js single row: Region badge escapes observer iata', () => { + const html = evalTemplate('public/packets.js', + /(\$\{region \?[\s\S]*?<\/td>)/, { region: PAYLOAD }); + assertNoXss(html, 'packets.js region'); +}); + +test('observers.js table row: Region badge escapes o.iata', () => { + const html = evalTemplate('public/observers.js', + /([\s\S]*?<\/td>)/, { o: { iata: PAYLOAD } }); + assertNoXss(html, 'observers.js row iata'); +}); + +test('observers.js slide-over: Region
escapes o.iata', () => { + const html = evalConcat('public/observers.js', + /'
Region<\/dt>
' \+ (\(o\.iata \? '' \+ [^;]*?\+ '<\/span>' : 'โ€”'\))/, + { o: { iata: PAYLOAD } }); + assertNoXss(html, 'observers.js slide-over iata'); +}); + +test('live.js node detail: "Heard By โ€” Regions" header escapes each iata', () => { + const html = evalTemplate('public/live.js', + /(

Heard By\$\{regions\.length[\s\S]*?<\/h4>)/, { regions: [PAYLOAD] }); + assertNoXss(html, 'live.js regions header'); +}); + +test('nodes.js clock-skew evidence: observer name/id is escaped', () => { + const src = fs.readFileSync('public/nodes.js', 'utf8'); + const m = src.match(/var name = ([^;]*o\.observerName \|\| o\.observerID[^;]*);/); + assert.ok(m, 'clock-skew observer name assignment not found'); + const fn = new Function('o', 'escapeHtml', 'return ' + m[1] + ';'); + const html = fn({ observerName: PAYLOAD }, escapeHtml); + assertNoXss(html, 'nodes.js clock-skew observer name'); +}); + +test('analytics.js RF Health cell: observer_id is escaped inside attributes', () => { + const html = evalTemplate('public/analytics.js', + /(
<\/div>)/, + { isSelected: false, obs: { observer_id: '"' + PAYLOAD }, esc: escapeHtml, + name: 'obs', nf: '-100', nfClass: '', batt: '' }); + assertNoXss(html, 'analytics.js rf-cell observer_id'); + // A raw double quote in the id would end the attribute and start a new one. + assert.ok(!/data-observer="[^"]*"\s*'?\s*onfocus/i.test(html), + 'raw " in observer_id broke out of data-observer attribute: ' + html); + assert.ok(html.includes('"'), 'double quote in observer_id was not escaped: ' + html); +}); + +test('app.js favorites dropdown: node name is escaped', () => { + const html = evalConcat('public/app.js', + /'' \+ (\S+\(h\.node\.name \|\| truncate\(pk, 12\)\)|\(h\.node\.name \|\| truncate\(pk, 12\)\)) \+ '<\/span>'/, + { h: { node: { name: PAYLOAD } }, pk: 'abcdef' }); + assertNoXss(html, 'app.js favorites node name'); +}); + +test('customize-v2.js prune preview: node name is escaped', () => { + const html = evalConcat('public/customize-v2.js', + /return '
' \+ (\S*\(n\.name \|\| n\.pubkey\.slice\(0, 12\)\)) \+ coords \+ '<\/div>';/, + { n: { name: PAYLOAD, pubkey: 'abcdef0123456789' } }); + assertNoXss(html, 'customize-v2.js prune preview node name'); +}); + // ========================================================================= // SUMMARY // =========================================================================