fix(security): close conversation hijacking, profile mass assignment, avatar MIME trust

src/app/api/auth/upload-avatar/route.js

@@ -1,5 +1,6 @@
 import { NextResponse } from 'next/server';
 import { createClient } from '@supabase/supabase-js';
+import { detectImageType } from '@/lib/server/detect-image-type.js';
 
 const supabaseAuth = createClient(process.env.NEXT_PUBLIC_SUPABASE_URL, process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY);
 
@@ -46,39 +47,34 @@ export async function POST(request) {
 			return NextResponse.json({ error: 'No file provided' }, { status: 400 });
 		}
 
-		// Validate file type
-		const allowedTypes = ['image/jpeg', 'image/png', 'image/webp', 'image/gif'];
-		if (!allowedTypes.includes(file.type)) {
-			return NextResponse.json({ 
-				error: 'Invalid file type. Please upload JPEG, PNG, WebP, or GIF images only.' 
-			}, { status: 400 });
-		}
-
 		// Validate file size (5MB limit)
 		if (file.size > 5 * 1024 * 1024) {
-			return NextResponse.json({ 
-				error: 'File size too large. Please upload files smaller than 5MB.' 
+			return NextResponse.json({
+				error: 'File size too large. Please upload files smaller than 5MB.'
 			}, { status: 400 });
 		}
 
-		// Generate unique filename
-		const fileExtByType = {
-			'image/jpeg': 'jpg',
-			'image/png': 'png',
-			'image/webp': 'webp',
-			'image/gif': 'gif'
-		};
-		const fileExt = fileExtByType[file.type];
-		const fileName = `${user.id}/${Date.now()}.${fileExt}`;
-
-		// Convert file to buffer for upload
+		// Convert file to buffer so the contents -- not the caller's claim about them --
+		// decide what this is.
 		const fileBuffer = await file.arrayBuffer();
 
+		// `file.type` is just a header the uploader chose, so an attacker could store
+		// arbitrary content under an image content-type and have it served back from the
+		// avatars bucket. Sniff the magic bytes and use only what they say.
+		const detected = detectImageType(new Uint8Array(fileBuffer));
+		if (!detected) {
+			return NextResponse.json({
+				error: 'Invalid file type. Please upload JPEG, PNG, WebP, or GIF images only.'
+			}, { status: 400 });
+		}
+
+		const fileName = `${user.id}/${Date.now()}.${detected.ext}`;
+
 		// Upload to Supabase Storage
 		const { error: uploadError } = await supabase.storage
 			.from('avatars')
 			.upload(fileName, fileBuffer, {
-				contentType: file.type,
+				contentType: detected.mime,
 				cacheControl: '3600',
 				upsert: false
 			});

src/lib/server/detect-image-type.js

@@ -0,0 +1,35 @@
+/**
+ * @fileoverview Identify an image by its magic bytes.
+ *
+ * An uploader controls both the filename and the Content-Type header, so neither can be
+ * allowed to decide what gets written into a public bucket and served back with an image
+ * content-type. Only the bytes decide.
+ */
+
+/**
+ * @param {Uint8Array} bytes
+ * @returns {{mime: string, ext: string} | null} null when it is not an allowed image
+ */
+export function detectImageType(bytes) {
+	if (!bytes || bytes.length < 12) return null;
+
+	const startsWith = (...sig) => sig.every((b, i) => bytes[i] === b);
+	const ascii = (offset, text) =>
+		[...text].every((ch, i) => bytes[offset + i] === ch.charCodeAt(0));
+
+	// JPEG: FF D8 FF
+	if (startsWith(0xff, 0xd8, 0xff)) return { mime: 'image/jpeg', ext: 'jpg' };
+
+	// PNG: 89 50 4E 47 0D 0A 1A 0A
+	if (startsWith(0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a)) {
+		return { mime: 'image/png', ext: 'png' };
+	}
+
+	// GIF: "GIF87a" or "GIF89a"
+	if (ascii(0, 'GIF8')) return { mime: 'image/gif', ext: 'gif' };
+
+	// WebP: "RIFF" .... "WEBP"
+	if (ascii(0, 'RIFF') && ascii(8, 'WEBP')) return { mime: 'image/webp', ext: 'webp' };
+
+	return null;
+}

src/lib/server/detect-image-type.test.js

@@ -0,0 +1,38 @@
+import { describe, expect, it } from 'vitest';
+import { detectImageType } from './detect-image-type.js';
+
+/** Build a 16-byte buffer starting with `head`, so length checks always pass. */
+function bytes(head) {
+	const out = new Uint8Array(16);
+	head.forEach((b, i) => (out[i] = b));
+	return out;
+}
+
+const ascii = (text) => [...text].map((c) => c.charCodeAt(0));
+
+describe('detectImageType', () => {
+	it('identifies the four allowed formats by magic bytes', () => {
+		expect(detectImageType(bytes([0xff, 0xd8, 0xff]))).toEqual({ mime: 'image/jpeg', ext: 'jpg' });
+		expect(detectImageType(bytes([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])))
+			.toEqual({ mime: 'image/png', ext: 'png' });
+		expect(detectImageType(bytes(ascii('GIF89a')))).toEqual({ mime: 'image/gif', ext: 'gif' });
+
+		const webp = new Uint8Array(16);
+		ascii('RIFF').forEach((b, i) => (webp[i] = b));
+		ascii('WEBP').forEach((b, i) => (webp[8 + i] = b));
+		expect(detectImageType(webp)).toEqual({ mime: 'image/webp', ext: 'webp' });
+	});
+
+	// The point of the change: a caller claiming image/png proves nothing.
+	it('rejects non-image content regardless of what the caller claims', () => {
+		expect(detectImageType(bytes(ascii('<?php system($_GET[0]);')))).toBeNull();
+		expect(detectImageType(bytes(ascii('<svg onload=alert(1)>')))).toBeNull();
+		expect(detectImageType(bytes([0x4d, 0x5a]))).toBeNull(); // PE executable
+	});
+
+	it('rejects input too short to carry a signature', () => {
+		expect(detectImageType(new Uint8Array([0xff, 0xd8, 0xff]))).toBeNull();
+		expect(detectImageType(new Uint8Array())).toBeNull();
+		expect(detectImageType(null)).toBeNull();
+	});
+});

supabase/migrations/20260816140000_close_conversation_hijack_and_profile_mass_assignment.sql

@@ -0,0 +1,104 @@
+-- Wave 3 of the 2026-08 security remediation.
+--
+-- Two findings from the consolidated assessment (GHSA-3hqc-9v44-j37g) that were
+-- still live in production:
+--
+--   V-007  Orphaned conversation hijacking. The UPDATE policy on `conversations`
+--          began `(created_by IS NULL) AND (auth.uid() IS NOT NULL)`, so any
+--          authenticated account could take over any conversation whose creator
+--          column was NULL -- rename it, or claim it by setting created_by.
+--   V-009  Mass assignment on `users`. `Users can update own profile` had a USING
+--          clause and no WITH CHECK and no column restriction, so a user could
+--          rewrite any column of their own row, including the identity-bearing
+--          `phone_number` and `unique_identifier`.
+--
+-- Also drops four UPDATE policies that compare `auth.uid() = id`. That is the
+-- identity domain drift recorded as IA-040: `auth.uid()` is the Supabase Auth
+-- UUID while `users.id` is the internal key. Verified against production before
+-- dropping -- 0 of 86 rows have `id = auth_user_id`, so these policies have never
+-- matched a single row and grant nothing.
+
+-- --------------------------------------------------------------------------
+-- V-007: a conversation may only be updated by its creator.
+--
+-- `conversations_update_policy` compared `created_by::text = auth.uid()::text`,
+-- which is the same domain drift as above and never matched either. Both are
+-- replaced by one policy that resolves the caller properly, and which carries a
+-- WITH CHECK so the new row cannot hand the conversation to someone else.
+-- --------------------------------------------------------------------------
+DROP POLICY IF EXISTS "Users can update own conversations" ON public.conversations;
+DROP POLICY IF EXISTS conversations_update_policy          ON public.conversations;
+DROP POLICY IF EXISTS conversations_update_creator         ON public.conversations;
+
+CREATE POLICY conversations_update_creator ON public.conversations
+    FOR UPDATE TO authenticated
+    USING      (created_by = public.current_app_user_id())
+    WITH CHECK (created_by = public.current_app_user_id());
+
+-- --------------------------------------------------------------------------
+-- V-009: pin the identity-bearing columns of `users`.
+--
+-- A column-level REVOKE was considered and rejected: it would have to enumerate
+-- every writable column correctly, and any column added later defaults back to
+-- writable. A trigger states the invariant directly and fails closed for columns
+-- nobody thought about.
+--
+-- service_role is exempt because the legitimate writers of these columns are all
+-- server-side and run as service_role: /api/auth/salt (salt),
+-- /api/auth/verify-sms and the CoinPay callback (phone_number),
+-- /api/auth/register-anon (unique_identifier).
+--
+-- The authenticated role legitimately writes only bio, website, updated_at and
+-- sms_notifications_enabled, none of which are pinned here.
+-- --------------------------------------------------------------------------
+CREATE OR REPLACE FUNCTION public.enforce_users_immutable_columns()
+RETURNS trigger
+LANGUAGE plpgsql
+SECURITY DEFINER
+SET search_path TO ''
+AS $function$
+BEGIN
+    IF current_setting('role', true) = 'service_role'
+       OR auth.uid() IS NULL THEN
+        RETURN NEW;
+    END IF;
+
+    IF NEW.id             IS DISTINCT FROM OLD.id
+       OR NEW.auth_user_id   IS DISTINCT FROM OLD.auth_user_id
+       OR NEW.phone_number   IS DISTINCT FROM OLD.phone_number
+       OR NEW.unique_identifier IS DISTINCT FROM OLD.unique_identifier
+       OR NEW.created_at     IS DISTINCT FROM OLD.created_at
+       OR NEW.salt           IS DISTINCT FROM OLD.salt
+    THEN
+        RAISE EXCEPTION
+            'Identity columns of users are not client-writable (id, auth_user_id, phone_number, unique_identifier, created_at, salt)';
+    END IF;
+
+    RETURN NEW;
+END;
+$function$;
+
+DROP TRIGGER IF EXISTS users_immutable_columns ON public.users;
+CREATE TRIGGER users_immutable_columns
+    BEFORE UPDATE ON public.users
+    FOR EACH ROW
+    EXECUTE FUNCTION public.enforce_users_immutable_columns();
+
+-- Give the surviving profile policy an explicit WITH CHECK so a row can never be
+-- updated out from under its owner, independent of the trigger above.
+DROP POLICY IF EXISTS "Users can update own profile" ON public.users;
+CREATE POLICY "Users can update own profile" ON public.users
+    FOR UPDATE TO authenticated
+    USING      (auth_user_id = auth.uid())
+    WITH CHECK (auth_user_id = auth.uid());
+
+-- --------------------------------------------------------------------------
+-- IA-040: drop the policies that can never match (auth.uid() = users.id).
+-- --------------------------------------------------------------------------
+DROP POLICY IF EXISTS "Users can update their own avatar_url"                  ON public.users;
+DROP POLICY IF EXISTS "Users can update their own disappearing messages settings" ON public.users;
+DROP POLICY IF EXISTS "Users can update their own profile fields"              ON public.users;
+DROP POLICY IF EXISTS "Users can read their own disappearing messages settings" ON public.users;
+
+COMMENT ON FUNCTION public.enforce_users_immutable_columns() IS
+    'Blocks client-side rewrites of identity columns on users (V-009). service_role is exempt.';