Skip to content

[CRITICAL] fix(security): scope legacy-key cleanup to authenticated user's own keys#76

Open
katnisscalls99 wants to merge 1 commit into
profullstack:masterfrom
katnisscalls99:fix/legacy-keys-missing-ownership-check
Open

[CRITICAL] fix(security): scope legacy-key cleanup to authenticated user's own keys#76
katnisscalls99 wants to merge 1 commit into
profullstack:masterfrom
katnisscalls99:fix/legacy-keys-missing-ownership-check

Conversation

@katnisscalls99
Copy link
Copy Markdown

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant