From 810801a9ab5df49cf47cd7bf9446fe6a54e6808d Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:30:13 +0200 Subject: [PATCH] fix(events): drop non-canonical keys from the event update before any check runs (#1346) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PUT /admin/events/:id spreads the body into the UPDATE. SQLite resolves quoted identifiers case-insensitively, so `{ "Event_Name": ... }` lands on event_name there — while every check in the handler (validators, the field-level permission guards, the deny-set) keys on the exact lowercase name. The deny-set already case-folded for its own columns; every other column was reachable through a spelling variant. Every events column and every input-only key the handler accepts is lowercase snake_case, so a key with any uppercase in it is not something a legitimate client sends. Such keys are now removed before anything looks at the body. Postgres was unaffected (quoted identifiers are case-sensitive there; a variant produced a 500 instead). Surfaced by the Codex review of the folder-watcher change, where a photos.upload guard on external_watch could be walked around this way. Co-authored-by: Paul Nothaft --- .../__tests__/routes/authzPermissionGaps.test.js | 8 +++++++- backend/src/routes/adminEvents/crud.js | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/backend/__tests__/routes/authzPermissionGaps.test.js b/backend/__tests__/routes/authzPermissionGaps.test.js index 46281d6b..0918590e 100644 --- a/backend/__tests__/routes/authzPermissionGaps.test.js +++ b/backend/__tests__/routes/authzPermissionGaps.test.js @@ -137,11 +137,17 @@ describe('authorization / ownership gaps', () => { // Case-variant keys — SQLite matches columns case-insensitively. Password_Hash: 'case-hijack-hash', Created_By: 88888, + // A case variant of an ORDINARY column must not reach the UPDATE + // either: field-level guards in the handler key on the exact name, + // and on SQLite the variant would still land on the real column. + Event_Name: 'case-variant-name', + Welcome_Message: 'case-variant-welcome', }); expect(res.status).toBe(200); const row = await db('events').where({ id: eventId }).first(); - expect(row.event_name).toBe('After'); // legit field applied + expect(row.event_name).toBe('After'); // legit field applied; Event_Name variant dropped + expect(row.welcome_message).toBeFalsy(); // case variant of an ordinary column dropped expect(row.created_by).toBe(superId); // ownership untouched (+ case-variant) expect(row.slug).toBe('authz-mass-assign'); // routing identity untouched expect(row.share_token).toBe(seedShareToken); // secret untouched diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index 58bb4b21..0c494422 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -1730,10 +1730,18 @@ module.exports = (router) => { // Legacy mirrors — rejected explicitly below in favour of customer_*. 'host_name', 'host_email', ]; - // Case-insensitive match: SQLite treats quoted identifiers - // case-insensitively, so a `{ "Password_Hash": ... }` key would - // otherwise survive a case-sensitive delete and still hit the real - // column (codex review). + // Only canonical keys reach the UPDATE. SQLite resolves quoted + // identifiers case-insensitively, so `{ "Event_Name": ... }` lands on + // event_name there while every check in this handler — validators, + // the permission guards on individual fields, the deny-set below — is + // keyed on the exact lowercase name. Every events column and every + // input-only key this handler accepts is lowercase snake_case, so a key + // with any uppercase in it is not something a legitimate client sends; + // it is dropped before anything looks at it. The deny-set keeps its own + // case-folding as belt and braces (GHSA-3rqx). + for (const key of Object.keys(updates)) { + if (key !== key.toLowerCase()) delete updates[key]; + } const denied = new Set(IMMUTABLE_EVENT_COLUMNS.map((c) => c.toLowerCase())); for (const key of Object.keys(updates)) { if (denied.has(key.toLowerCase())) delete updates[key];