From 0f00624e61aebe17c7819fbd94efab4b2e4bade7 Mon Sep 17 00:00:00 2001 From: Steve Dogiakos Date: Thu, 19 Mar 2026 22:44:11 -0600 Subject: [PATCH] Fix three critical authorization vulnerabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mark-printed: was only checking the first check's account — now fetches all check IDs upfront, verifies they all exist and share the same account, then checks editor access once on that account. PDF generation: was authorizing against the client-supplied account_id but fetching checks by ID without confirming they belong to that account — now rejects any check ID whose account_id doesn't match. Role/account-assignment changes: active sessions for the affected user are now deleted immediately via json_extract on the sessions table, so demotions take effect at once rather than at session expiry (up to 7d). --- src/routes/checks.js | 17 +++++++++++++---- src/routes/pdf.js | 9 ++++----- src/routes/users.js | 7 +++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/routes/checks.js b/src/routes/checks.js index f5e79f9..8fbd340 100644 --- a/src/routes/checks.js +++ b/src/routes/checks.js @@ -138,12 +138,21 @@ router.post('/mark-printed', (req, res) => { if (!Array.isArray(ids) || ids.length === 0) { return res.status(400).json({ error: 'ids array required' }); } - // Verify editor access via the first check's account - const first = db.prepare('SELECT account_id FROM checks WHERE id = ?').get(ids[0]); - if (!first || !isEditorForAccount(req.session, first.account_id)) { + + // Fetch all checks and verify they all exist and belong to the same account + const placeholders = ids.map(() => '?').join(','); + const rows = db.prepare(`SELECT id, account_id FROM checks WHERE id IN (${placeholders})`).all(...ids); + if (rows.length !== ids.length) { + return res.status(404).json({ error: 'One or more checks not found.' }); + } + const accountIds = [...new Set(rows.map(r => r.account_id))]; + if (accountIds.length > 1) { + return res.status(400).json({ error: 'All checks must belong to the same account.' }); + } + if (!isEditorForAccount(req.session, accountIds[0])) { return res.status(403).json({ error: 'Write access required.' }); } - const placeholders = ids.map(() => '?').join(','); + db.prepare(`UPDATE checks SET printed = 1 WHERE id IN (${placeholders})`).run(...ids); res.json({ updated: ids.length }); }); diff --git a/src/routes/pdf.js b/src/routes/pdf.js index 0166458..8f30346 100644 --- a/src/routes/pdf.js +++ b/src/routes/pdf.js @@ -20,24 +20,23 @@ router.post('/', async (req, res) => { if (!Array.isArray(checkIds) || checkIds.length === 0) { return res.status(400).json({ error: 'checkIds must be a non-empty array' }); } - if (!isEditorForAccount(req.session, parseInt(account_id, 10))) { + const resolvedAccountId = parseInt(account_id, 10); + if (!isEditorForAccount(req.session, resolvedAccountId)) { return res.status(403).json({ error: 'Write access required.' }); } - // Fetch checks in the order provided + // Fetch checks in the order provided; verify each belongs to the declared account let checks; try { checks = checkIds.map(id => { const check = db.prepare('SELECT * FROM checks WHERE id = ?').get(id); if (!check) throw new Error(`Check ID ${id} not found`); + if (check.account_id !== resolvedAccountId) throw new Error(`Check ID ${id} does not belong to this account`); return check; }); } catch (err) { return res.status(404).json({ error: err.message }); } - - // Derive account from checks (all should belong to the same account) - const resolvedAccountId = account_id || checks[0].account_id; const account = db.prepare('SELECT * FROM account WHERE id = ?').get(resolvedAccountId); if (!account) { return res.status(500).json({ error: 'No account configured.' }); diff --git a/src/routes/users.js b/src/routes/users.js index e56e288..6c55998 100644 --- a/src/routes/users.js +++ b/src/routes/users.js @@ -95,6 +95,13 @@ router.put('/:id', async (req, res) => { } } + // If role or account assignments changed, invalidate all active sessions for this user + // so the new permissions take effect immediately rather than at session expiry. + if (role || Array.isArray(accounts)) { + db.prepare("DELETE FROM sessions WHERE CAST(json_extract(sess, '$.userId') AS INTEGER) = ?") + .run(parseInt(req.params.id, 10)); + } + res.json(userWithAccounts(req.params.id)); });