From d5304e1d7976dc8e565a42b2052c41a07d749b9a Mon Sep 17 00:00:00 2001 From: katnisscalls99 Date: Sat, 6 Jun 2026 05:09:24 -0700 Subject: [PATCH] fix(security): scope legacy-key cleanup to authenticated user's own keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/app/api/cleanup/legacy-keys/route.js | 102 +++++++++++++---------- 1 file changed, 58 insertions(+), 44 deletions(-) diff --git a/src/app/api/cleanup/legacy-keys/route.js b/src/app/api/cleanup/legacy-keys/route.js index 2ebc8a2..e53b250 100644 --- a/src/app/api/cleanup/legacy-keys/route.js +++ b/src/app/api/cleanup/legacy-keys/route.js @@ -1,103 +1,117 @@ import { NextResponse } from 'next/server'; -import { createSupabaseClient } from '@/lib/supabase.js'; +import { createSupabaseServerClient } from '@/lib/supabase.js'; +import { createServiceRoleClient } from '@/lib/supabase/service-role.js'; export async function DELETE(request) { try { - // Get supabase client (will use cookies automatically) - const supabase = createSupabaseClient(); - - // Verify user is authenticated with better error handling - const { data: { session } } = await supabase.auth.getSession(); - - if (!session || !session.user) { - console.error('Authentication failed - no valid session found'); + // Use createSupabaseServerClient (reads cookies server-side) and validate + // with getUser() rather than getSession() to ensure the JWT is re-verified + // against the Supabase Auth server and cannot be spoofed via cookie tampering. + const supabase = await createSupabaseServerClient(); + const { data: { user }, error: authError } = await supabase.auth.getUser(); + + if (authError || !user) { + console.error('Authentication failed - no valid user found'); return NextResponse.json({ error: 'Unauthorized - No valid session', details: 'Please login again' }, { status: 401 }); } - - const userId = session.user.id; - console.log(`Authenticated user ${userId} requesting legacy key cleanup`); - - // Find user public keys that are not ML-KEM-1024 format - // We can detect this by looking at the key size - ML-KEM-1024 keys are 1568 bytes - // when decoded from Base64 - const { data: legacyKeys, error: findError } = await supabase + + // Resolve the internal user ID from the Supabase Auth UUID. + const serviceRoleClient = createServiceRoleClient(); + const { data: internalUser, error: internalUserError } = await serviceRoleClient + .from('users') + .select('id') + .eq('auth_user_id', user.id) + .single(); + + if (internalUserError || !internalUser) { + console.error('User record not found for auth_user_id:', user.id); + return NextResponse.json({ error: 'User not found' }, { status: 404 }); + } + + const internalUserId = internalUser.id; + console.log(`Authenticated user ${internalUserId} requesting legacy key cleanup (own keys only)`); + + // SECURITY FIX: Scope the query to the authenticated user's keys ONLY. + // Previously the query fetched ALL user_public_keys without a user_id filter, + // allowing any authenticated user to delete other users' public keys — a + // privilege-escalation / denial-of-service against the entire user base. + const { data: legacyKeys, error: findError } = await serviceRoleClient .from('user_public_keys') - .select('*'); - + .select('*') + .eq('user_id', internalUserId); + if (findError) { console.error('Error finding legacy keys:', findError); return NextResponse.json({ error: findError.message }, { status: 500 }); } - + // Function to check if a key is valid ML-KEM-1024 format const isValidMlKem1024Key = (/** @type {any} */ key) => { try { // Skip if no public_key if (!key.public_key) return false; - + // Check for obvious ML-KEM-1024 marker if (key.public_key.includes('ML-KEM-1024')) return true; - + // Estimate decoded size by base64 length // Base64 encodes 3 bytes in 4 characters, so decode length ≈ (base64Length * 3) / 4 const estimatedBytes = (key.public_key.length * 3) / 4; - + // ML-KEM-1024 keys are 1568 bytes when decoded // Use a range to account for some variation const isSizeValid = estimatedBytes >= 1550 && estimatedBytes <= 1590; - + // Additional format checks for more aggressive detection - // Invalid keys often have unusual patterns or characters - // 1. Check for invalid base64 characters const hasInvalidChars = /[^A-Za-z0-9+/=]/.test(key.public_key); - + // 2. Check for patterns that indicate corrupted keys const hasCorruptedPattern = key.public_key.includes('null') || key.public_key.includes('undefined') || key.public_key.includes('[object'); - + // 3. Check for keys that are too clean (all the same character) const tooManyRepeats = /(.)\1{20,}/.test(key.public_key); - - // A valid key must have the right size and not trigger any of the warning flags + return isSizeValid && !hasInvalidChars && !hasCorruptedPattern && !tooManyRepeats; } catch (e) { console.error('Error checking key format:', e); return false; } }; - - // Filter for non-ML-KEM-1024 keys + + // Filter for non-ML-KEM-1024 keys belonging to the authenticated user const nonMlKem1024Keys = legacyKeys.filter(key => !isValidMlKem1024Key(key)); - - console.log(`Found ${nonMlKem1024Keys.length} legacy keys out of ${legacyKeys.length} total keys`); - + + console.log(`Found ${nonMlKem1024Keys.length} legacy keys out of ${legacyKeys.length} total keys for user ${internalUserId}`); + if (nonMlKem1024Keys.length === 0) { return NextResponse.json({ message: 'No legacy keys found', updatedCount: 0 }); } - - // For each legacy key, delete it from the database - // The user will need to reset their key to the proper format + + // Delete only the authenticated user's legacy keys. + // The extra .eq('user_id', internalUserId) is a defence-in-depth guard. let deletedCount = 0; - + for (const key of nonMlKem1024Keys) { - const { error: deleteError } = await supabase + const { error: deleteError } = await serviceRoleClient .from('user_public_keys') .delete() - .eq('id', key.id); - + .eq('id', key.id) + .eq('user_id', internalUserId); // defence-in-depth ownership check + if (deleteError) { console.error(`Error deleting legacy key ${key.id}:`, deleteError); } else { deletedCount++; } } - + return NextResponse.json({ message: 'Successfully deleted legacy keys', deletedCount, @@ -110,4 +124,4 @@ export async function DELETE(request) { { status: 500 } ); } -} \ No newline at end of file +}