HIGH: Payment UPDATE/DELETE lack user_id in WHERE clause (defense-in-depth) #72

New Issue

Closed

opened 2026-05-31 12:03:07 -05:00 by null · 1 comment

null commented

2026-05-31 12:03:07 -05:00

Owner

Bug Description

After verifying ownership via a JOIN query, the subsequent UPDATE and DELETE operations on payments use only the payment id in the WHERE clause, without including user_id:

UPDATE payments SET ... WHERE id = ?
DELETE FROM payments WHERE id = ?

The ownership check happens first via a JOIN (payments JOIN bills WHERE b.user_id = ?), so this is not currently exploitable. However, its a defense-in-depth violation:

TOCTOU window: Between the ownership check and the mutation, the payment could theoretically be reassigned
The final SELECT after UPDATE/restore returns raw payment data without re-verifying ownership

Affected Code

routes/payments.js:413-427 (PUT /:id - UPDATE without user_id)
routes/payments.js:433 (DELETE /:id - DELETE without user_id)
routes/payments.js:453-454 (POST /:id/restore - UPDATE and SELECT without user_id)
routes/payments.js:414 (SELECT * FROM payments WHERE id = ? without user_id)

Impact

Defense-in-depth gap. If the initial ownership check were ever bypassed or code restructured, payments could be modified across user boundaries.

Recommended Fix

Add AND bill_id IN (SELECT id FROM bills WHERE user_id = ?) to all payment UPDATE/DELETE statements, or re-fetch the payment through the ownership JOIN after mutation.

## Bug Description After verifying ownership via a JOIN query, the subsequent UPDATE and DELETE operations on payments use only the payment id in the WHERE clause, without including user_id: UPDATE payments SET ... WHERE id = ? DELETE FROM payments WHERE id = ? The ownership check happens first via a JOIN (payments JOIN bills WHERE b.user_id = ?), so this is not currently exploitable. However, its a defense-in-depth violation: 1. TOCTOU window: Between the ownership check and the mutation, the payment could theoretically be reassigned 2. The final SELECT after UPDATE/restore returns raw payment data without re-verifying ownership ## Affected Code - routes/payments.js:413-427 (PUT /:id - UPDATE without user_id) - routes/payments.js:433 (DELETE /:id - DELETE without user_id) - routes/payments.js:453-454 (POST /:id/restore - UPDATE and SELECT without user_id) - routes/payments.js:414 (SELECT * FROM payments WHERE id = ? without user_id) ## Impact Defense-in-depth gap. If the initial ownership check were ever bypassed or code restructured, payments could be modified across user boundaries. ## Recommended Fix Add AND bill_id IN (SELECT id FROM bills WHERE user_id = ?) to all payment UPDATE/DELETE statements, or re-fetch the payment through the ownership JOIN after mutation.