mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-29 16:12:06 -04:00
fix(security): escape backslashes in calendar bg-image CSS url() (#4712)
* 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:<url> 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).
This commit is contained in:
@@ -12,7 +12,7 @@ import {
|
|||||||
WEEKDAYS, WEEKDAYS_SUN, MONTHS, MON_SHORT,
|
WEEKDAYS, WEEKDAYS_SUN, MONTHS, MON_SHORT,
|
||||||
CAL_PALETTE, CAL_COLORS, _CAL_CUSTOM_GRADIENT, _TYPE_PALETTE,
|
CAL_PALETTE, CAL_COLORS, _CAL_CUSTOM_GRADIENT, _TYPE_PALETTE,
|
||||||
_trashIcon, _moreIcon, _bellIcon,
|
_trashIcon, _moreIcon, _bellIcon,
|
||||||
_isCalBgImage, _calBgImageUrl, _calBgCss,
|
_isCalBgImage, _calBgImageUrl, _calBgCss, _cssUrlEscape,
|
||||||
_calReadableTextColor,
|
_calReadableTextColor,
|
||||||
_ds, _addDays, _shiftDT, _tzOffset, _localDateOf,
|
_ds, _addDays, _shiftDT, _tzOffset, _localDateOf,
|
||||||
} from './calendar/utils.js';
|
} from './calendar/utils.js';
|
||||||
@@ -413,8 +413,8 @@ function _calEventFg(ev) {
|
|||||||
// Returns '' for normal solid-color events.
|
// Returns '' for normal solid-color events.
|
||||||
function _calItemBgStyle(ev) {
|
function _calItemBgStyle(ev) {
|
||||||
if (!_isCalBgImage(ev.color)) return '';
|
if (!_isCalBgImage(ev.color)) return '';
|
||||||
const url = _calBgImageUrl(ev.color).replace(/'/g, "\\'").replace(/"/g, "%22");
|
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('${url}'); background-size: cover; background-position: center;`;
|
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() {
|
function _todayCount() {
|
||||||
@@ -1260,8 +1260,8 @@ async function _renderWeek() {
|
|||||||
// events keep the original tinted treatment.
|
// events keep the original tinted treatment.
|
||||||
let bgDecl;
|
let bgDecl;
|
||||||
if (_isCalBgImage(ev.color)) {
|
if (_isCalBgImage(ev.color)) {
|
||||||
const _url = _calBgImageUrl(ev.color).replace(/'/g, "\\'").replace(/"/g, "%22");
|
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('${_url}'); background-size: cover; background-position: center;`;
|
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 {
|
} else {
|
||||||
bgDecl = `background:color-mix(in srgb, ${_calColor(ev)} 18%, var(--bg));`;
|
bgDecl = `background:color-mix(in srgb, ${_calColor(ev)} 18%, var(--bg));`;
|
||||||
}
|
}
|
||||||
@@ -2853,7 +2853,7 @@ function _showEventForm(existing, defaultDate, defaultEndDate) {
|
|||||||
let bg;
|
let bg;
|
||||||
if (isCustom) {
|
if (isCustom) {
|
||||||
const url = _calBgImageUrl(cur);
|
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 {
|
} else {
|
||||||
bg = c.hex || 'var(--border)';
|
bg = c.hex || 'var(--border)';
|
||||||
}
|
}
|
||||||
@@ -2928,7 +2928,7 @@ function _showEventForm(existing, defaultDate, defaultEndDate) {
|
|||||||
// stays readable. Chrome accent falls back to the theme accent.
|
// stays readable. Chrome accent falls back to the theme accent.
|
||||||
const url = _calBgImageUrl(hex);
|
const url = _calBgImageUrl(hex);
|
||||||
_formCard.style.setProperty('--ev-color', 'var(--accent)');
|
_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.backgroundSize = 'cover';
|
||||||
_formCard.style.backgroundPosition = 'center';
|
_formCard.style.backgroundPosition = 'center';
|
||||||
_formCard.classList.add('cal-form-bg-image');
|
_formCard.classList.add('cal-form-bg-image');
|
||||||
@@ -2950,7 +2950,7 @@ function _showEventForm(existing, defaultDate, defaultEndDate) {
|
|||||||
if (!url) return;
|
if (!url) return;
|
||||||
const sentinel = 'bg:' + url;
|
const sentinel = 'bg:' + url;
|
||||||
dot.dataset.color = sentinel;
|
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'));
|
document.querySelectorAll('#cal-f-colors .note-color-dot').forEach(d => d.classList.remove('active'));
|
||||||
dot.classList.add('active');
|
dot.classList.add('active');
|
||||||
_applyFormTint(sentinel);
|
_applyFormTint(sentinel);
|
||||||
|
|||||||
@@ -65,13 +65,25 @@ export function _calBgImageUrl(c) {
|
|||||||
return _isCalBgImage(c) ? c.slice(3) : '';
|
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
|
// 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
|
// 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).
|
// too small to render usefully (small grid dots, multi-day bars).
|
||||||
export function _calBgCss(c, fallback) {
|
export function _calBgCss(c, fallback) {
|
||||||
if (_isCalBgImage(c)) {
|
if (_isCalBgImage(c)) {
|
||||||
const u = _calBgImageUrl(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)';
|
return c || fallback || 'var(--accent)';
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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)
|
||||||
|
)
|
||||||
Reference in New Issue
Block a user