From 2f246c77796a2e3a34981e7e34f18c534ba3131c Mon Sep 17 00:00:00 2001 From: nopoz Date: Mon, 22 Jun 2026 12:17:52 -0700 Subject: [PATCH] fix(security): escape backslashes in calendar bg-image CSS url() (#4712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): escape backslashes in calendar bg-image CSS url() The calendar event-background CSS escaped ' -> \' for a bg: image URL but not backslashes first. Inside a single-quoted url('...'), \ is the CSS escape char, so a URL value ending in/containing a backslash escapes the closing quote and breaks out of the string, injecting arbitrary CSS. The bg: value is per-event and CalDAV-syncable, hence untrusted (CodeQL js/incomplete-sanitization). Add a single canonical _cssUrlEscape() in calendar/utils.js that escapes backslashes FIRST, then quotes, and route all four sinks through it: calendar.js:416 / :1263 (the flagged #463/#464), the event-form preview (:2931), and _calBgCss() in utils.js — the latter two share the identical bug but were unflagged. Output is byte-identical to the old escaping for legitimate URLs (which contain no backslashes); only malicious input differs. Resolves CodeQL js/incomplete-sanitization #463, #464. * fix(security): route remaining calendar bg url() sinks through _cssUrlEscape Review (vdmkenny) flagged that the centralization missed an injectable sibling sink: the edit-form color-picker swatch (calendar.js:2856) built `url('${url}')` from `existing.color` (a CalDAV-syncable, untrusted `bg:` value) raw, then interpolated it into `style="background:..."` via innerHTML - the same `'`/`\` breakout class as the sinks already fixed. The custom-dot preview (:2953) was likewise raw (non-exploitable - a CSSOM `.style` assignment of a URL the current user just picked - but it broke the invariant). Route both through `_cssUrlEscape`, and normalize the two pre-escaped-variable sites (_calItemBgStyle, _renderWeek) to the same inline form so all five url() interpolations in calendar.js follow one rule. Add a whole-file invariant test asserting every `url('${...}')` calls `_cssUrlEscape` - this catches a future missed sink, the exact failure mode here. Behavior-identical for legitimate URLs (no visual change). --- static/js/calendar.js | 16 ++-- static/js/calendar/utils.js | 14 ++- tests/test_calendar_css_url_escape_js.py | 107 +++++++++++++++++++++++ 3 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 tests/test_calendar_css_url_escape_js.py diff --git a/static/js/calendar.js b/static/js/calendar.js index 2b14d024a..26a235523 100644 --- a/static/js/calendar.js +++ b/static/js/calendar.js @@ -12,7 +12,7 @@ import { WEEKDAYS, WEEKDAYS_SUN, MONTHS, MON_SHORT, CAL_PALETTE, CAL_COLORS, _CAL_CUSTOM_GRADIENT, _TYPE_PALETTE, _trashIcon, _moreIcon, _bellIcon, - _isCalBgImage, _calBgImageUrl, _calBgCss, + _isCalBgImage, _calBgImageUrl, _calBgCss, _cssUrlEscape, _calReadableTextColor, _ds, _addDays, _shiftDT, _tzOffset, _localDateOf, } from './calendar/utils.js'; @@ -413,8 +413,8 @@ function _calEventFg(ev) { // Returns '' for normal solid-color events. function _calItemBgStyle(ev) { if (!_isCalBgImage(ev.color)) return ''; - const url = _calBgImageUrl(ev.color).replace(/'/g, "\\'").replace(/"/g, "%22"); - return `background-image: linear-gradient(color-mix(in srgb, var(--bg) 70%, transparent), color-mix(in srgb, var(--bg) 70%, transparent)), url('${url}'); background-size: cover; background-position: center;`; + const url = _calBgImageUrl(ev.color); + return `background-image: linear-gradient(color-mix(in srgb, var(--bg) 70%, transparent), color-mix(in srgb, var(--bg) 70%, transparent)), url('${_cssUrlEscape(url)}'); background-size: cover; background-position: center;`; } function _todayCount() { @@ -1260,8 +1260,8 @@ async function _renderWeek() { // events keep the original tinted treatment. let bgDecl; if (_isCalBgImage(ev.color)) { - const _url = _calBgImageUrl(ev.color).replace(/'/g, "\\'").replace(/"/g, "%22"); - bgDecl = `background-image: linear-gradient(color-mix(in srgb, var(--bg) 55%, transparent), color-mix(in srgb, var(--bg) 55%, transparent)), url('${_url}'); background-size: cover; background-position: center;`; + const _url = _calBgImageUrl(ev.color); + bgDecl = `background-image: linear-gradient(color-mix(in srgb, var(--bg) 55%, transparent), color-mix(in srgb, var(--bg) 55%, transparent)), url('${_cssUrlEscape(_url)}'); background-size: cover; background-position: center;`; } else { bgDecl = `background:color-mix(in srgb, ${_calColor(ev)} 18%, var(--bg));`; } @@ -2853,7 +2853,7 @@ function _showEventForm(existing, defaultDate, defaultEndDate) { let bg; if (isCustom) { const url = _calBgImageUrl(cur); - bg = url ? `center/cover no-repeat url('${url}')` : _CAL_CUSTOM_GRADIENT; + bg = url ? `center/cover no-repeat url('${_cssUrlEscape(url)}')` : _CAL_CUSTOM_GRADIENT; } else { bg = c.hex || 'var(--border)'; } @@ -2928,7 +2928,7 @@ function _showEventForm(existing, defaultDate, defaultEndDate) { // stays readable. Chrome accent falls back to the theme accent. const url = _calBgImageUrl(hex); _formCard.style.setProperty('--ev-color', 'var(--accent)'); - _formCard.style.backgroundImage = `linear-gradient(color-mix(in srgb, var(--panel) 65%, transparent), color-mix(in srgb, var(--panel) 65%, transparent)), url('${url.replace(/'/g, "\\'")}')`; + _formCard.style.backgroundImage = `linear-gradient(color-mix(in srgb, var(--panel) 65%, transparent), color-mix(in srgb, var(--panel) 65%, transparent)), url('${_cssUrlEscape(url)}')`; _formCard.style.backgroundSize = 'cover'; _formCard.style.backgroundPosition = 'center'; _formCard.classList.add('cal-form-bg-image'); @@ -2950,7 +2950,7 @@ function _showEventForm(existing, defaultDate, defaultEndDate) { if (!url) return; const sentinel = 'bg:' + url; dot.dataset.color = sentinel; - dot.style.background = `center/cover no-repeat url('${url}')`; + dot.style.background = `center/cover no-repeat url('${_cssUrlEscape(url)}')`; document.querySelectorAll('#cal-f-colors .note-color-dot').forEach(d => d.classList.remove('active')); dot.classList.add('active'); _applyFormTint(sentinel); diff --git a/static/js/calendar/utils.js b/static/js/calendar/utils.js index 7e6dd68e8..a0743cc12 100644 --- a/static/js/calendar/utils.js +++ b/static/js/calendar/utils.js @@ -65,13 +65,25 @@ export function _calBgImageUrl(c) { return _isCalBgImage(c) ? c.slice(3) : ''; } +// Escape a value for safe embedding inside a single-quoted CSS `url('...')`. +// Backslashes MUST be escaped first: otherwise a trailing/embedded `\` in the +// (CalDAV-syncable, untrusted) bg-image URL would escape the closing quote we +// add for `'` and let the value break out of the string (CodeQL +// js/incomplete-sanitization). `"` is percent-encoded for good measure. +export function _cssUrlEscape(s) { + return String(s == null ? '' : s) + .replace(/\\/g, '\\\\') + .replace(/'/g, "\\'") + .replace(/"/g, '%22'); +} + // Returns a value safe to drop into `style="background:..."`. Falls back to // the calendar default for bg-image events in spots where an image would be // too small to render usefully (small grid dots, multi-day bars). export function _calBgCss(c, fallback) { if (_isCalBgImage(c)) { const u = _calBgImageUrl(c); - return u ? `center/cover no-repeat url('${u.replace(/'/g, "\\'")}')` : (fallback || 'var(--accent)'); + return u ? `center/cover no-repeat url('${_cssUrlEscape(u)}')` : (fallback || 'var(--accent)'); } return c || fallback || 'var(--accent)'; } diff --git a/tests/test_calendar_css_url_escape_js.py b/tests/test_calendar_css_url_escape_js.py new file mode 100644 index 000000000..ad188c6d4 --- /dev/null +++ b/tests/test_calendar_css_url_escape_js.py @@ -0,0 +1,107 @@ +r"""DOM/CSS-injection regression for calendar background-image URL escaping. + +CodeQL `js/incomplete-sanitization` (#463 calendar.js:416, #464 calendar.js:1263) +flagged event-background CSS that escaped `'` -> `\'` without first escaping +backslashes. A `bg:`-color value (settable per event, and CalDAV-syncable, so +untrusted) ending in or containing a backslash can then consume the closing +quote of `url('...')` and break out of the CSS string. + +The fix is a single canonical escaper, `_cssUrlEscape`, in calendar/utils.js, +used by both inline sinks and by `_calBgCss` (which had the same incomplete +escaping). These tests pin the escaper: backslashes are doubled FIRST, then +quotes, so no input can terminate the `url('...')` string early. +""" + +import json +import re +import shutil +import subprocess +import textwrap +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parent.parent +_UTILS = (_REPO / "static" / "js" / "calendar" / "utils.js").as_posix() +_CALENDAR_JS = _REPO / "static" / "js" / "calendar.js" +_HAS_NODE = shutil.which("node") is not None + +pytestmark = pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH") + + +def _run(js: str) -> str: + proc = subprocess.run( + ["node", "--input-type=module"], + input=js, capture_output=True, text=True, cwd=str(_REPO), timeout=30, + ) + assert proc.returncode == 0, proc.stderr + return proc.stdout.strip() + + +def test_cssurlescape_doubles_backslashes_before_quotes(): + js = textwrap.dedent( + f""" + const {{ _cssUrlEscape }} = await import('{_UTILS}'); + console.log(JSON.stringify({{ + backslash: _cssUrlEscape('a\\\\b'), + trailing: _cssUrlEscape('img\\\\'), + quote: _cssUrlEscape("a'b"), + dquote: _cssUrlEscape('a"b'), + }})); + """ + ) + out = json.loads(_run(js)) + # one backslash -> two; the escape for "'" is not itself re-escaped + assert out["backslash"] == r"a\\b" + assert out["trailing"] == "img\\\\" # 'img\' -> 'img\\' + assert out["quote"] == r"a\'b" + assert out["dquote"] == "a%22b" + + +def test_backslash_breakout_payload_cannot_close_the_url_string(): + # Without the backslash-first escape, "x\" would render url('x\') and the + # trailing backslash escapes the closing quote -> breakout. After the fix the + # backslash is doubled, so the quote we add still terminates the string. + js = textwrap.dedent( + f""" + const {{ _cssUrlEscape, _calBgCss }} = await import('{_UTILS}'); + const payload = 'x\\\\'; // a string ending in one backslash + console.log(JSON.stringify({{ + esc: _cssUrlEscape(payload), + css: _calBgCss('bg:' + payload, 'var(--accent)'), + }})); + """ + ) + out = json.loads(_run(js)) + assert out["esc"] == "x\\\\" # doubled backslash + # The rendered declaration keeps the backslash doubled inside url('...'). + assert "url('x\\\\')" in out["css"] + + +def test_calbgcss_escapes_quote_breakout(): + js = textwrap.dedent( + f""" + const {{ _calBgCss }} = await import('{_UTILS}'); + console.log(JSON.stringify(_calBgCss("bg:a'); X{{}}//", 'var(--accent)'))); + """ + ) + css = json.loads(_run(js)) + # the injected single quote is escaped, so the url() string is not closed early + assert r"\'" in css + assert "url('a\\'); X{}//')" in css + + +def test_every_calendar_url_interpolation_is_escaped(): + # Whole-file invariant: every CSS `url('${...}')` built in calendar.js must + # route its (CalDAV-syncable, untrusted) value through `_cssUrlEscape`. This + # is the guard that catches a *newly added* bg-image sink the centralization + # forgot - the failure mode that left calendar.js:2856 (edit-form color + # swatch) and :2953 (custom-dot preview) raw before this change. + src = _CALENDAR_JS.read_text(encoding="utf-8") + interps = re.findall(r"url\('\$\{([^}]*)\}'\)", src) + assert interps, "expected at least one url('${...}') interpolation in calendar.js" + unescaped = [expr for expr in interps if "_cssUrlEscape(" not in expr] + assert not unescaped, ( + "bg-image url() interpolation(s) not routed through _cssUrlEscape: " + + ", ".join(repr(e) for e in unescaped) + )