[CRITICAL] fix(security): scope legacy-key cleanup to authenticated user's own keys#76
Open
katnisscalls99 wants to merge 1 commit into
Conversation
VULNERABILITY: Any authenticated user could call DELETE /api/cleanup/legacy-keys
and delete the public keys of every other user on the platform.
Root cause:
1. getSession() was used instead of getUser(); getSession() reads the session
from the cookie without re-validating the JWT against the Supabase Auth server,
making it potentially bypassable by cookie tampering.
2. The user_public_keys query had NO user_id filter — it fetched ALL rows from
the table and then deleted any row whose key failed the ML-KEM-1024 heuristic.
The 'userId' variable was logged but never used in the DB query.
Impact:
- An attacker with a valid session token could wipe every user's public key,
breaking end-to-end encryption for the entire user base.
- Affected users' future messages would be undeliverable (no key to encrypt to).
- In-flight messages encrypted to the deleted keys would be undecryptable.
Fix:
- Switch to createSupabaseServerClient + getUser() for server-side JWT validation.
- Resolve the internal user ID via auth_user_id.
- Add .eq('user_id', internalUserId) filter to the SELECT query so only the
calling user's keys are evaluated and deleted.
- Add the same ownership filter to each DELETE call as defence-in-depth.
Severity: CRITICAL — privilege escalation / cross-user data destruction
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
VULNERABILITY: Any authenticated user could call
DELETE /api/cleanup/legacy-keysand delete the public keys of every other user on the platform.Root cause:
getSession()was used instead ofgetUser();getSession()reads the session from the cookie without re-validating the JWT against the Supabase Auth server, making it potentially bypassable by cookie tampering.user_public_keysquery had NO user_id filter — it fetched ALL rows from the table and then deleted any row whose key failed the ML-KEM-1024 heuristic. TheuserIdvariable was logged but never used in the DB query.Impact:
Fix:
createSupabaseServerClient+getUser()for server-side JWT validation.auth_user_id..eq('user_id', internalUserId)filter to the SELECT query so only the calling user's keys are evaluated and deleted.Severity: CRITICAL — privilege escalation / cross-user data destruction