From bf70c3d7d7f5110bf04334abf198a5a17102d24f Mon Sep 17 00:00:00 2001 From: Maxim Dolgolyov Date: Sun, 17 May 2026 00:30:34 +0300 Subject: [PATCH] =?UTF-8?q?fix(admin-redesign):=20security=20=E2=80=94=20s?= =?UTF-8?q?tored=20XSS=20via=20user=20name=20in=20onclick?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security review caught: per-row hover actions (users.js) and async user picker (shop.js, gam.js) interpolated user-controlled name into JS string literals inside onclick. LS.esc() escapes & < > " but NOT backslash; the .replace(/'/g, '\'') fallback was broken. Attack: any authenticated user could set their name to a\'); alert(1); // via PATCH /api/auth/profile (stripTags doesn't strip \) — admin viewing the users/shop/gam picker would execute arbitrary JS. Fix: switch from JS-string interpolation to data-uid/data-name attributes, read via dataset in handler. esc() correctly escapes for HTML-attribute context; dataset returns the raw string with zero parse re-entry. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/js/admin/sections/gam.js | 12 +++++++----- frontend/js/admin/sections/shop.js | 11 ++++++----- frontend/js/admin/sections/users.js | 29 +++++++++++++++++++++-------- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/frontend/js/admin/sections/gam.js b/frontend/js/admin/sections/gam.js index 4f957d4..8ad7d19 100644 --- a/frontend/js/admin/sections/gam.js +++ b/frontend/js/admin/sections/gam.js @@ -118,17 +118,19 @@ _gamSearchTimer = setTimeout(async () => { try { const r = await LS.adminGetUsers({ q, limit: 8 }); - box.innerHTML = (r.users || []).map(u => `
- ${esc(u.name || u.email)}${u.role} + const label = u => u.name || u.email; + box.innerHTML = (r.users || []).map(u => `
+ ${esc(label(u))}${esc(u.role)}
`).join('') || '
Не найдено
'; box.classList.add('open'); } catch(e) { box.classList.remove('open'); } }, 300); } - function gamPickUser(id, name, prefix) { - document.getElementById(prefix + '-uid').value = id; - document.getElementById(prefix + '-user').value = name; + function gamPickUser(el) { + const prefix = el.dataset.prefix; + document.getElementById(prefix + '-uid').value = el.dataset.uid; + document.getElementById(prefix + '-user').value = el.dataset.name || ''; document.getElementById(prefix + '-results').classList.remove('open'); } diff --git a/frontend/js/admin/sections/shop.js b/frontend/js/admin/sections/shop.js index 13e1174..a880d27 100644 --- a/frontend/js/admin/sections/shop.js +++ b/frontend/js/admin/sections/shop.js @@ -156,17 +156,18 @@ _shopSearchTimer = setTimeout(async () => { try { const r = await LS.adminGetUsers({ q, limit: 8 }); - box.innerHTML = (r.users || []).map(u => `
- ${esc(u.name || u.email)}${u.role} + const label = u => u.name || u.email; + box.innerHTML = (r.users || []).map(u => `
+ ${esc(label(u))}${esc(u.role)}
`).join('') || '
Не найдено
'; box.classList.add('open'); } catch(e) { box.classList.remove('open'); } }, 300); } - function shopPickUser(id, name) { - document.getElementById('shop-award-uid').value = id; - document.getElementById('shop-award-user').value = name; + function shopPickUser(el) { + document.getElementById('shop-award-uid').value = el.dataset.uid; + document.getElementById('shop-award-user').value = el.dataset.name || ''; document.getElementById('shop-award-results').classList.remove('open'); } diff --git a/frontend/js/admin/sections/users.js b/frontend/js/admin/sections/users.js index 4058180..dbd8bb1 100644 --- a/frontend/js/admin/sections/users.js +++ b/frontend/js/admin/sections/users.js @@ -119,19 +119,27 @@ } const banIcon = u.is_banned ? ICONS.unlock : ICONS.ban; const banLabel = u.is_banned ? 'Разблокировать' : 'Заблокировать'; + // Pass uid/name via data-* attributes (esc() escapes & < > " for the attribute + // context; dataset reads back the raw string — no JS-string injection surface). return `
+ data-uid="${u.id}" data-banned="${u.is_banned?1:0}" + onclick="event.stopPropagation();quickToggleBan(this)">${banIcon} + data-uid="${u.id}" data-name="${esc(u.name)}" + onclick="event.stopPropagation();quickAwardCoins(this)">${ICONS.coins} + data-uid="${u.id}" + onclick="event.stopPropagation();quickOpenUserSessions(this)">${ICONS.history} + data-uid="${u.id}" data-name="${esc(u.name)}" + onclick="event.stopPropagation();quickDeleteUser(this)">${ICONS.trash}
`; } - async function quickToggleBan(uid, isBanned, btn) { + async function quickToggleBan(btn) { + const uid = +btn.dataset.uid; + const isBanned = +btn.dataset.banned; const action = isBanned ? 'Разблокировать' : 'Заблокировать'; const msg = isBanned ? 'Разблокировать пользователя? Он снова сможет войти в систему.' @@ -152,7 +160,9 @@ } } - function quickAwardCoins(uid, name) { + function quickAwardCoins(btn) { + const uid = +btn.dataset.uid; + const name = btn.dataset.name || ''; const body = document.createElement('div'); body.innerHTML = `

Начислить монеты пользователю ${esc(name)}:

@@ -187,7 +197,8 @@ setTimeout(() => body.querySelector('#qa-coins-amt')?.focus(), 80); } - function quickOpenUserSessions(uid) { + function quickOpenUserSessions(btn) { + const uid = +btn.dataset.uid; // Phase 6: open the user's deep page with the Sessions sub-tab active. if (window.AdminRouter) AdminRouter.navigate('#users/' + uid + '/sessions'); else if (typeof window.switchTab === 'function') { @@ -196,7 +207,9 @@ } } - async function quickDeleteUser(uid, name, btn) { + async function quickDeleteUser(btn) { + const uid = +btn.dataset.uid; + const name = btn.dataset.name || ''; if (!await LS.confirm( `Удалить пользователя «${name}» навсегда?\nВсе его данные, тесты и прогресс будут удалены. Это действие нельзя отменить.`, { title: 'Удалить пользователя', confirmText: 'Удалить навсегда' }