From ca8ca38a322d13238b0c51461f151b40b45bcaca Mon Sep 17 00:00:00 2001 From: Vykos Date: Thu, 4 Jun 2026 20:51:23 +0200 Subject: [PATCH] Guard image and QR DOM attributes (#2500) --- static/js/notes.js | 30 ++++++++++++++------- static/js/settings.js | 9 +++++-- static/js/signature.js | 34 +++++++++++++++++++----- tests/test_notes_dom_xss_helpers.py | 34 ++++++++++++++++++++++++ tests/test_signature_settings_dom_xss.py | 26 ++++++++++++++++++ 5 files changed, 114 insertions(+), 19 deletions(-) create mode 100644 tests/test_notes_dom_xss_helpers.py create mode 100644 tests/test_signature_settings_dom_xss.py diff --git a/static/js/notes.js b/static/js/notes.js index 6bd0ccc..935b6b7 100644 --- 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, '>'); } -// 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 . +function _attrEsc(s) { + return String(s || '') + .replace(/"/g, '"') + .replace(/'/g, ''') + .replace(//g, '>') + .replace(/`/g, '`'); +} +// 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 . 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 `${url}` + (url !== m ? m.slice(url.length) : ''); + return `${url}` + (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 = `
@@ -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'); diff --git a/static/js/settings.js b/static/js/settings.js index dd9240f..161f722 100644 --- 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 = `
- QR Code + ${qrCode ? `QR Code` : ''}
Scan with your authenticator app, or enter manually:
-
${setup.secret}
+
${esc(setup.secret)}
diff --git a/static/js/signature.js b/static/js/signature.js index 36780f7..94f8dfe 100644 --- 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, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + +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) => ` -
- -
${(s.name || '').replace(/[<>&]/g, '')}
- + const tiles = sigs.map((s) => { + const dataUrl = _safeSignatureDataUrl(s.data_url); + if (!dataUrl) return ''; + return ` +
+ +
${_esc(s.name || '')}
+
- `).join(''); + `; + }).join(''); const overlay = _modal(`
@@ -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); } diff --git a/tests/test_notes_dom_xss_helpers.py b/tests/test_notes_dom_xss_helpers.py new file mode 100644 index 0000000..92e5d3d --- /dev/null +++ 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 diff --git a/tests/test_signature_settings_dom_xss.py b/tests/test_signature_settings_dom_xss.py new file mode 100644 index 0000000..daa3388 --- /dev/null +++ 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 '' 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 '${setup.secret}
" not in src