12 of ~16 hand-rolled CSRF checks omit csrf_token from their rejection response, breaking client-side resync app-wide #85

New Issue

2026-09-01T00:08:16-04:00

jared commented

2026-09-01 00:08:16 -04:00

Severity: High

api/custom_fields.php, clone_ticket.php, delete_comment.php, delete_attachment.php, bulk_operation.php, generate_api_key.php, manage_templates.php, manage_recurring.php, revoke_api_key.php, manage_workflows.php, ticket_dependencies.php, and upload_attachment.php all duplicate CsrfMiddleware::validateToken() inline instead of routing through api/bootstrap.php, and their 403 rejection body is just {"success":false,"error":"Invalid CSRF token"} — no csrf_token field.

Compare to api/bootstrap.php (~lines 40-47) and its direct users (add_comment.php, update_ticket.php), which correctly echo CsrfMiddleware::getToken() on rejection specifically so lt.api's resync (assets/js/base.js:475) can recover.

Impact: Once window.CSRF_TOKEN goes stale for any reason (token expiry after ~1h idle — see related CSRF-retry issue for the exact numbers, or a write in another tab rotating the shared session-scoped token), the next write against any of these 12 endpoints returns a 403 with no recovery token in the body. The client has no way to self-heal at that point — every subsequent write to any endpoint (even ones that would normally resync fine) keeps failing until the user manually reloads the page. This is strictly broader and more severe than the already-filed #57 (notification bell's raw fetch(), one specific endpoint) — the resync mechanism is effectively only implemented on a minority of the app's write surface.

Fix: Either route all 12 of these endpoints through bootstrap.php's centralized CSRF handling (preferred — also fixes error-shape/display_errors inconsistencies other audit rounds already flagged), or at minimum add 'csrf_token' => CsrfMiddleware::getToken() to each of these 12 rejection bodies.

**Severity:** High `api/custom_fields.php`, `clone_ticket.php`, `delete_comment.php`, `delete_attachment.php`, `bulk_operation.php`, `generate_api_key.php`, `manage_templates.php`, `manage_recurring.php`, `revoke_api_key.php`, `manage_workflows.php`, `ticket_dependencies.php`, and `upload_attachment.php` all duplicate `CsrfMiddleware::validateToken()` inline instead of routing through `api/bootstrap.php`, and their 403 rejection body is just `{"success":false,"error":"Invalid CSRF token"}` — no `csrf_token` field. Compare to `api/bootstrap.php` (~lines 40-47) and its direct users (`add_comment.php`, `update_ticket.php`), which correctly echo `CsrfMiddleware::getToken()` on rejection specifically so `lt.api`'s resync (`assets/js/base.js:475`) can recover. **Impact:** Once `window.CSRF_TOKEN` goes stale for any reason (token expiry after ~1h idle — see related CSRF-retry issue for the exact numbers, or a write in another tab rotating the shared session-scoped token), the *next* write against any of these 12 endpoints returns a 403 with no recovery token in the body. The client has no way to self-heal at that point — every subsequent write to *any* endpoint (even ones that would normally resync fine) keeps failing until the user manually reloads the page. This is strictly broader and more severe than the already-filed #57 (notification bell's raw `fetch()`, one specific endpoint) — the resync mechanism is effectively only implemented on a minority of the app's write surface. **Fix:** Either route all 12 of these endpoints through `bootstrap.php`'s centralized CSRF handling (preferred — also fixes error-shape/display_errors inconsistencies other audit rounds already flagged), or at minimum add `'csrf_token' => CsrfMiddleware::getToken()` to each of these 12 rejection bodies.

jared added the priority/high security labels 2026-09-08 10:15:49 -04:00

jared referenced this issue from a commit

2026-09-11 11:43:00 -04:00

Add recovery csrf_token to 12 hand-rolled CSRF rejection responses (#85)

jared referenced this issue from a commit

2026-09-11 11:54:37 -04:00

Merge development into main: connection/security hardening batch (#85, #94, #103, #104)

jared commented

2026-09-11 11:54:44 -04:00

Fixed and verified against real MariaDB and real running endpoints (session-based CSRF flow, health-check states, connection charset/timezone sync). Merged to main in commit bcc732e.

Fixed and verified against real MariaDB and real running endpoints (session-based CSRF flow, health-check states, connection charset/timezone sync). Merged to main in commit bcc732e.

jared closed this issue

2026-09-11 11:54:45 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: LotusGuild/tinker_tickets#85