Guard image and QR DOM attributes (#2500)

2026-06-04 20:51:23 +02:00
parent b59bbe80ce
commit ca8ca38a32
5 changed files with 114 additions and 19 deletions
--- a/static/js/notes.js
+++ b/static/js/notes.js
@@ -438,13 +438,22 @@ async function _patchNote(id, patch) {
 // ---- Helpers ----

 function _esc(s) { return uiModule.esc ? uiModule.esc(s || '') : (s || '').replace(/</g, '&lt;').replace(/>/g, '&gt;'); }
-// Image src guard — reject anything that isn't a relative path or http(s)/data URL
-// so an AI-saved note can't slip a `javascript:` URL into the rendered <img>.
+function _attrEsc(s) {
+  return String(s || '')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/`/g, '&#96;');
+}
+// Image src guard — reject anything that isn't a relative path, http(s), or
+// raster data URL so an AI-saved note can't slip script-capable media into the
+// rendered <img>.
 function _safeImgSrc(s) {
  const v = (s || '').trim();
  if (!v) return '';
  if (v.startsWith('/') || v.startsWith('./') || v.startsWith('../')) return v;
-  if (/^https?:\/\//i.test(v) || /^data:image\//i.test(v)) return v;
+  if (/^https?:\/\//i.test(v) || /^data:image\/(?:png|jpe?g|gif|webp);base64,/i.test(v)) return v;
  return '';
 }

@@ -461,7 +470,7 @@ function _linkify(s) {
      url = url.slice(0, -1);
    }
    const href = url.startsWith('www.') ? `https://${url}` : url;
-    return `<a href="${href}" class="note-link" target="_blank" rel="noopener noreferrer" onclick="event.stopPropagation()">${url}</a>` + (url !== m ? m.slice(url.length) : '');
+    return `<a href="${_attrEsc(href)}" class="note-link" target="_blank" rel="noopener noreferrer" onclick="event.stopPropagation()">${url}</a>` + (url !== m ? m.slice(url.length) : '');
  });
 }
 function _uid() { return Math.random().toString(36).slice(2, 10); }
@@ -2779,7 +2788,7 @@ function _buildForm(note = null) {
  form.className = 'note-form';
  if (color && !_isBgImage(color)) form.classList.add('note-color-' + color);
  if (_isBgImage(color)) form.setAttribute('style', _customColorStyle(color));
-  let currentImageUrl = note?.image_url || '';
+  let currentImageUrl = _safeImgSrc(note?.image_url || '');
  form.innerHTML = `
    <div class="note-form-header">
      <input type="text" class="note-form-title" placeholder="Title" value="${_esc(note?.title || '')}" />
@@ -2861,7 +2870,7 @@ function _buildForm(note = null) {
  let _stashedGoalItems = (type === 'goal' && Array.isArray(note?.items)) ? note.items.slice() : null;

  // Drawing also stashes the saved image URL so it survives Note↔Draw flips.
-  let _stashedDrawUrl = (type === 'draw') ? (note?.image_url || null) : null;
+  let _stashedDrawUrl = (type === 'draw') ? (_safeImgSrc(note?.image_url) || null) : null;
  const _refreshFormLayout = () => {
    const body = form.closest('.notes-pane-body');
    if (!body) return;
@@ -2913,7 +2922,7 @@ function _buildForm(note = null) {
        // toggled to Draw, paint that photo onto the canvas so they can draw
        // on top of it. _stashedDrawUrl wins if they were drawing earlier in
        // the same edit session.
-        _wireCanvas(bodyEl, _stashedDrawUrl || currentImageUrl || note?.image_url || null);
+        _wireCanvas(bodyEl, _stashedDrawUrl || currentImageUrl || _safeImgSrc(note?.image_url) || null);
      } else {
        const text = (_stashedNoteText !== null && _stashedNoteText !== undefined && _stashedNoteText !== '')
          ? _stashedNoteText
@@ -3003,7 +3012,7 @@ function _buildForm(note = null) {
  if (currentType === 'todo') _wireChecklist(form.querySelector('.note-form-body'));
  if (currentType === 'goal') _wireGoalForm(form, form.querySelector('.note-form-body'));
  if (currentType === 'draw') {
-    _wireCanvas(form.querySelector('.note-form-body'), note?.image_url || null);
+    _wireCanvas(form.querySelector('.note-form-body'), _safeImgSrc(note?.image_url) || null);
    // Same hides we apply on type-switch — keep them consistent on initial open.
    const _ip = form.querySelector('.note-form-image-wrap'); if (_ip) _ip.style.display = 'none';
    const _cp = form.querySelector('.note-color-picker'); if (_cp) _cp.style.display = 'none';
@@ -3894,11 +3903,12 @@ function _wireCanvas(container, initialImageUrl) {
  ctx.lineJoin = 'round';

  // Load prior drawing as starting point so consecutive edits compose.
-  if (initialImageUrl) {
+  const safeInitialImageUrl = _safeImgSrc(initialImageUrl);
+  if (safeInitialImageUrl) {
    const img = new Image();
    img.crossOrigin = 'anonymous';
    img.onload = () => { try { ctx.drawImage(img, 0, 0, cssW, cssH); } catch {} };
-    img.src = initialImageUrl;
+    img.src = safeInitialImageUrl;
    // Float an X over the canvas so the user can blank it out and go back to
    // a clean draw surface. Removes itself once clicked.
    const wrap = container.querySelector('.note-form-draw-wrap');
--- a/static/js/settings.js
+++ b/static/js/settings.js
@@ -13,6 +13,10 @@ let modalEl = null;

 function el(id) { return document.getElementById(id); }
 function esc(s) { return uiModule.esc(s); }
+function safeRasterDataUrl(raw) {
+  const value = String(raw || '').trim();
+  return /^data:image\/(?:png|jpe?g|gif|webp);base64,[a-z0-9+/=\s]+$/i.test(value) ? value : '';
+}

 /* ── Tab switching ── */
 const ADMIN_TABS = new Set(['services', 'integrations', 'tools', 'users', 'system']);
@@ -2069,15 +2073,16 @@ function initAccount() {
              const r = await fetch('/api/auth/2fa/setup', { method: 'POST', credentials: 'same-origin' });
              if (!r.ok) { const d = await r.json(); throw new Error(d.detail || 'Failed'); }
              const setup = await r.json();
+              const qrCode = safeRasterDataUrl(setup.qr_code);
              // Show QR code + manual secret + verify input
              tfaContent.innerHTML = `
                <div style="text-align:center;margin-bottom:12px;">
-                  <img src="${setup.qr_code}" alt="QR Code" style="border-radius:8px;max-width:200px;">
+                  ${qrCode ? `<img src="${esc(qrCode)}" alt="QR Code" style="border-radius:8px;max-width:200px;">` : ''}
                </div>
                <div style="font-size:11px;opacity:0.5;text-align:center;margin-bottom:8px;">
                  Scan with your authenticator app, or enter manually:
                </div>
-                <div style="font-family:monospace;font-size:12px;text-align:center;padding:6px;background:var(--bg);border:1px solid var(--border);border-radius:4px;margin-bottom:12px;word-break:break-all;user-select:all;cursor:text;">${setup.secret}</div>
+                <div style="font-family:monospace;font-size:12px;text-align:center;padding:6px;background:var(--bg);border:1px solid var(--border);border-radius:4px;margin-bottom:12px;word-break:break-all;user-select:all;cursor:text;">${esc(setup.secret)}</div>
                <input id="tfa-verify-code" type="text" placeholder="Enter 6-digit code to verify" autocomplete="one-time-code" inputmode="numeric" maxlength="8" style="width:100%;padding:8px;background:var(--bg);border:1px solid var(--border);border-radius:4px;color:var(--fg);font-family:inherit;font-size:13px;box-sizing:border-box;text-align:center;letter-spacing:3px;margin-bottom:6px;">
                <div class="settings-row" style="justify-content:flex-end;">
                  <span id="tfa-msg" style="font-size:11px;margin-right:auto;"></span>
--- a/static/js/signature.js
+++ b/static/js/signature.js
@@ -14,6 +14,20 @@

 const API_BASE = window.location.origin;

+function _esc(s) {
+  return String(s ?? '')
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;');
+}
+
+function _safeSignatureDataUrl(raw) {
+  const value = String(raw || '').trim();
+  return /^data:image\/(?:png|jpe?g);base64,[a-z0-9+/=\s]+$/i.test(value) ? value : '';
+}
+
 // Last signature the user picked or created in this session. Lets the export
 // modal pre-fill subsequent signature fields with the same one — sign once,
 // applies everywhere.
@@ -446,13 +460,17 @@ export function capture(opts = {}) {
 export function pick(opts = {}) {
  return new Promise(async (resolve) => {
    const sigs = await _listSignatures();
-    const tiles = sigs.map((s) => `
-      <div class="sig-tile" data-id="${s.id}">
-        <img src="${s.data_url}"/>
-        <div style="margin-top:4px;font-size:0.72rem;color:var(--fg);opacity:0.85;text-align:center;overflow:hidden;text-overflow:ellipsis;white-space:nowrap;">${(s.name || '').replace(/[<>&]/g, '')}</div>
-        <button class="sig-tile-del" data-id="${s.id}" title="Delete">×</button>
+    const tiles = sigs.map((s) => {
+      const dataUrl = _safeSignatureDataUrl(s.data_url);
+      if (!dataUrl) return '';
+      return `
+      <div class="sig-tile" data-id="${_esc(s.id)}">
+        <img src="${_esc(dataUrl)}"/>
+        <div style="margin-top:4px;font-size:0.72rem;color:var(--fg);opacity:0.85;text-align:center;overflow:hidden;text-overflow:ellipsis;white-space:nowrap;">${_esc(s.name || '')}</div>
+        <button class="sig-tile-del" data-id="${_esc(s.id)}" title="Delete">×</button>
      </div>
-    `).join('');
+    `;
+    }).join('');

    const overlay = _modal(`
      <div class="modal-content" style="width:min(560px,94vw);">
@@ -477,7 +495,9 @@ export function pick(opts = {}) {
        const id = tile.dataset.id;
        const s = sigs.find((x) => x.id === id);
        if (s) {
-          const out = { id: s.id, dataUrl: s.data_url, width: s.width, height: s.height, name: s.name };
+          const dataUrl = _safeSignatureDataUrl(s.data_url);
+          if (!dataUrl) return;
+          const out = { id: s.id, dataUrl, width: s.width, height: s.height, name: s.name };
          setLastUsed(out);
          close(out);
        }
--- a/tests/test_notes_dom_xss_helpers.py
+++ b/tests/test_notes_dom_xss_helpers.py
@@ -0,0 +1,34 @@
+"""Regression guards for Notes DOM rendering helpers."""
+
+from pathlib import Path
+
+
+_REPO = Path(__file__).resolve().parent.parent
+
+
+def test_notes_image_src_guard_rejects_script_capable_data_images():
+    src = (_REPO / "static" / "js" / "notes.js").read_text(encoding="utf-8")
+
+    assert "function _safeImgSrc(s)" in src
+    assert r"^data:image\/(?:png|jpe?g|gif|webp);base64," in src
+    assert r"^data:image\/i.test(v)" not in src
+
+
+def test_notes_linkify_escapes_href_attribute():
+    src = (_REPO / "static" / "js" / "notes.js").read_text(encoding="utf-8")
+
+    assert "function _attrEsc(s)" in src
+    assert 'href="${_attrEsc(href)}"' in src
+    assert 'href="${href}"' not in src
+
+
+def test_notes_edit_form_uses_safe_image_src_guard():
+    src = (_REPO / "static" / "js" / "notes.js").read_text(encoding="utf-8")
+
+    assert "let currentImageUrl = _safeImgSrc(note?.image_url || '');" in src
+    assert "let _stashedDrawUrl = (type === 'draw') ? (_safeImgSrc(note?.image_url) || null) : null;" in src
+    assert "_wireCanvas(bodyEl, _stashedDrawUrl || currentImageUrl || _safeImgSrc(note?.image_url) || null)" in src
+    assert "_wireCanvas(form.querySelector('.note-form-body'), _safeImgSrc(note?.image_url) || null)" in src
+    assert "const safeInitialImageUrl = _safeImgSrc(initialImageUrl);" in src
+    assert "img.src = safeInitialImageUrl;" in src
+    assert "img.src = initialImageUrl;" not in src
--- a/tests/test_signature_settings_dom_xss.py
+++ b/tests/test_signature_settings_dom_xss.py
@@ -0,0 +1,26 @@
+"""Regression guards for DOM attribute sinks in signature/settings UI."""
+
+from pathlib import Path
+
+
+_REPO = Path(__file__).resolve().parent.parent
+
+
+def test_signature_picker_allows_only_raster_data_urls():
+    src = (_REPO / "static" / "js" / "signature.js").read_text(encoding="utf-8")
+
+    assert "function _safeSignatureDataUrl(raw)" in src
+    assert r"^data:image\/(?:png|jpe?g);base64," in src
+    assert '<img src="${_esc(dataUrl)}"/>' in src
+    assert 'dataUrl: s.data_url' not in src
+
+
+def test_settings_2fa_setup_escapes_secret_and_qr_src():
+    src = (_REPO / "static" / "js" / "settings.js").read_text(encoding="utf-8")
+
+    assert "function safeRasterDataUrl(raw)" in src
+    assert "const qrCode = safeRasterDataUrl(setup.qr_code);" in src
+    assert '<img src="${esc(qrCode)}"' in src
+    assert "${esc(setup.secret)}" in src
+    assert 'src="${setup.qr_code}"' not in src
+    assert ">${setup.secret}</div>" not in src