Skip to content

Security: dashboard builds HTML via innerHTML with unescaped device names (stored XSS defense-in-depth) #19

Description

@LarsLaskowski

Source: Project review 2026-07-13 (Performance / Security / Robustness), finding S2.
Category: security · Severity: medium · Effort: small

Problem

internal/web/assets/app.js builds DOM rows by string-concatenating server-provided values into innerHTML:

  1. barRow(name, ...) (around lines 226–235) interpolates the filesystem mountpoint both into a title="..." attribute and into element text:
row.innerHTML =
  '<div class="bar-label"><span class="bar-name" title="' + name + '">' + name + '</span>' + ...
  1. The network card renderer (inside renderMetrics, around lines 135–143) interpolates the interface name n.name into innerHTML the same way.

Mountpoints and interface names are not guaranteed to be HTML-safe. A mountpoint can contain almost arbitrary characters (e.g. a USB stick auto-mounted under a label like /media/pi/``&lt;img src=x onerror=...&gt;'``), and would then execute script in the browser of anyone viewing the dashboard. Since the dashboard is typically open unauthenticated on the LAN, this is a realistic stored-XSS vector even though it requires some local influence on mount names. The updates modal (renderUpdatesTable) already does this correctly with document.createElement+textContent` — the bar rows should follow the same pattern.

Fix

In internal/web/assets/app.js:

  1. Rewrite barRow(name, pct, warn, crit, subText) to build its elements with document.createElement(...) and assign all dynamic strings via .textContent (and el.title = name for the tooltip — property assignment, not attribute string). Set the fill width via el.style.width and classes via className. No innerHTML with interpolated data.
  2. Rewrite the network row renderer in renderMetrics the same way (n.name, and the formatted rx/tx strings, via textContent).
  3. Static markup with no interpolated data may keep using innerHTML, but the simplest rule is: no innerHTML anywhere except clearing (container.innerHTML = '' is fine).

Acceptance criteria

  • grep -n "innerHTML" internal/web/assets/app.js shows no remaining occurrence that interpolates dynamic data (clearing with '' is acceptable).
  • Dashboard renders identically for normal data (bar rows show name, percent, sub-text, colored fill of the right width; network rows show name and rates).
  • A mountpoint string like '&lt;img src=x onerror=alert(1)&gt;' renders as literal text, not as an element (manual test or a small DOM unit test if a JS test setup is added).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions