Skip to content

Commit fae949d

Browse files
ralyodioclaude
ralyodio
and
claude
authored
fix(security): bind the remaining IDOR-able RPCs to auth.uid() (#248)
20260814032259 stopped anon reaching these, but they still trusted an id passed as a parameter, so any authenticated user could act on another user's rows. The most serious was fn_mark_message_read: marking another user's message read also starts their disappearing-message timer, so it destroyed data rather than just flipping state. Also closed: fn_get_user_active_deliveries (read receipts and expiry metadata for any user), fn_create_deliveries_for_message, archive/unarchive_conversation, and update_user_activity. fn_cleanup_expired_messages is locked to service_role -- both callers already use that client and a NULL argument triggers a global GC pass. Corrects an earlier over-claim: find_user_by_unique_identifier is NOT an IDOR. It returns only public profile columns and is a lookup by design, so it is left alone. log_sms_notification is left open on purpose. It is genuinely cross-user (the sender logs on behalf of the recipient), so an auth.uid() binding would break it; the real fix is passing the service-role client in the websocket handler, which is a code change rather than a migration. Already applied to production as 20260814032942. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f0552cd commit fae949d

1 file changed

Lines changed: 256 additions & 0 deletions

File tree

‎supabase/migrations/20260814032942_bind_remaining_idor_rpcs_to_auth_uid.sql‎

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
-- Binds the remaining IDOR-able SECURITY DEFINER functions to auth.uid().
2+
-- Context: docs/audits/2026-08-14-noir0x63-verification.md
3+
--
4+
-- 20260814032259 stopped `anon` reaching these, but they still trusted an id
5+
-- passed as a parameter, so any *authenticated* user could act on another
6+
-- user's rows. This binds each to the session instead.
7+
--
8+
-- The `auth.uid() IS NOT NULL AND ...` shape lets service_role (no JWT, so a
9+
-- NULL auth.uid()) keep working; `anon` is already revoked, so the only other
10+
-- caller is `authenticated`, which always has a non-NULL auth.uid().
11+
--
12+
-- Bodies are reproduced from the live definitions; only the guard is new.
13+
--
14+
-- Impact ranking of what this closes:
15+
-- fn_mark_message_read - marking another user's message read also
16+
-- STARTS THEIR DISAPPEARING-MESSAGE TIMER,
17+
-- so this destroyed data, not just state.
18+
-- fn_get_user_active_deliveries - read receipts / delivery + expiry
19+
-- metadata for any user.
20+
-- fn_create_deliveries_for_message - fan out deliveries for someone else's
21+
-- message.
22+
-- archive_conversation /
23+
-- unarchive_conversation - archive state on another user's
24+
-- conversation (user_uuid here is the
25+
-- AUTH user id, hence the direct compare).
26+
-- update_user_activity - mark an arbitrary user online.
27+
--
28+
-- Also locks fn_cleanup_expired_messages to service_role: both callers
29+
-- (lib/services/message-cleanup-service.js) already use the service-role
30+
-- client, and with a NULL argument the function performs a GLOBAL GC pass.
31+
--
32+
-- DELIBERATELY NOT CHANGED:
33+
-- find_user_by_unique_identifier - previously listed as an IDOR, which was
34+
-- wrong. It returns only public profile columns (id, username,
35+
-- display_name, avatar_url, bio, website) and is a lookup by design, gated
36+
-- at api/users/by-id. No ownership relationship to enforce.
37+
-- log_sms_notification - genuinely cross-user: the sender logs a
38+
-- notification on behalf of the recipient, so an auth.uid() binding would
39+
-- break it. The right fix is to pass the service-role client in
40+
-- lib/websocket/handlers/messages.js (it already imports one) and then
41+
-- lock the function down. That is a code change, not a migration, so it is
42+
-- left open rather than half-done here.
43+
44+
CREATE OR REPLACE FUNCTION public.archive_conversation(conversation_uuid uuid, user_uuid uuid)
45+
RETURNS boolean
46+
LANGUAGE plpgsql
47+
SECURITY DEFINER
48+
SET search_path TO ''
49+
AS $function$
50+
DECLARE
51+
internal_user_id UUID;
52+
BEGIN
53+
IF auth.uid() IS NOT NULL AND user_uuid <> auth.uid() THEN
54+
RETURN FALSE;
55+
END IF;
56+
57+
SELECT id INTO internal_user_id
58+
FROM public.users
59+
WHERE auth_user_id::text = user_uuid::text;
60+
61+
IF internal_user_id IS NULL THEN
62+
RETURN FALSE;
63+
END IF;
64+
65+
UPDATE public.conversation_participants
66+
SET archived_at = NOW()
67+
WHERE conversation_id = conversation_uuid
68+
AND user_id = internal_user_id
69+
AND left_at IS NULL
70+
AND archived_at IS NULL;
71+
72+
RETURN FOUND;
73+
END;
74+
$function$;
75+
76+
CREATE OR REPLACE FUNCTION public.unarchive_conversation(conversation_uuid uuid, user_uuid uuid)
77+
RETURNS boolean
78+
LANGUAGE plpgsql
79+
SECURITY DEFINER
80+
SET search_path TO ''
81+
AS $function$
82+
DECLARE
83+
internal_user_id UUID;
84+
BEGIN
85+
IF auth.uid() IS NOT NULL AND user_uuid <> auth.uid() THEN
86+
RETURN FALSE;
87+
END IF;
88+
89+
SELECT id INTO internal_user_id
90+
FROM public.users
91+
WHERE auth_user_id::text = user_uuid::text;
92+
93+
IF internal_user_id IS NULL THEN
94+
RETURN FALSE;
95+
END IF;
96+
97+
UPDATE public.conversation_participants
98+
SET archived_at = NULL
99+
WHERE conversation_id = conversation_uuid
100+
AND user_id = internal_user_id
101+
AND left_at IS NULL
102+
AND archived_at IS NOT NULL;
103+
104+
RETURN FOUND;
105+
END;
106+
$function$;
107+
108+
CREATE OR REPLACE FUNCTION public.update_user_activity(user_uuid uuid)
109+
RETURNS void
110+
LANGUAGE plpgsql
111+
SECURITY DEFINER
112+
SET search_path TO ''
113+
AS $function$
114+
BEGIN
115+
IF auth.uid() IS NOT NULL AND NOT EXISTS (
116+
SELECT 1 FROM public.users u
117+
WHERE u.id = user_uuid AND u.auth_user_id = auth.uid()
118+
) THEN
119+
RETURN;
120+
END IF;
121+
122+
UPDATE public.users
123+
SET
124+
last_active_at = NOW(),
125+
is_online = TRUE
126+
WHERE id = user_uuid;
127+
END;
128+
$function$;
129+
130+
CREATE OR REPLACE FUNCTION public.fn_get_user_active_deliveries(p_user_id uuid)
131+
RETURNS TABLE(message_id uuid, delivered_ts timestamp with time zone, read_ts timestamp with time zone, expires_at timestamp with time zone)
132+
LANGUAGE sql
133+
SECURITY DEFINER
134+
SET search_path TO ''
135+
AS $function$
136+
SELECT
137+
d.message_id,
138+
d.delivered_ts,
139+
d.read_ts,
140+
d.expires_at
141+
FROM public.deliveries d
142+
WHERE d.recipient_user_id = p_user_id
143+
AND d.deleted_ts IS NULL
144+
AND (
145+
auth.uid() IS NULL
146+
OR EXISTS (
147+
SELECT 1 FROM public.users u
148+
WHERE u.id = p_user_id AND u.auth_user_id = auth.uid()
149+
)
150+
);
151+
$function$;
152+
153+
CREATE OR REPLACE FUNCTION public.fn_mark_message_read(p_message_id uuid, p_user_id uuid)
154+
RETURNS void
155+
LANGUAGE plpgsql
156+
SECURITY DEFINER
157+
SET search_path TO ''
158+
AS $function$
159+
DECLARE
160+
v_disappear_seconds INTEGER;
161+
v_start_on TEXT;
162+
v_expires_at TIMESTAMP WITH TIME ZONE;
163+
BEGIN
164+
IF auth.uid() IS NOT NULL AND NOT EXISTS (
165+
SELECT 1 FROM public.users u
166+
WHERE u.id = p_user_id AND u.auth_user_id = auth.uid()
167+
) THEN
168+
RETURN;
169+
END IF;
170+
171+
SELECT
172+
CASE
173+
WHEN cp.disappear_seconds IS NOT NULL THEN cp.disappear_seconds
174+
ELSE 0
175+
END,
176+
COALESCE(mr.start_on, 'delivered')
177+
INTO v_disappear_seconds, v_start_on
178+
FROM public.message_recipients mr
179+
JOIN public.messages m ON m.id = mr.message_id
180+
JOIN public.conversation_participants cp ON cp.conversation_id = m.conversation_id AND cp.user_id = mr.recipient_user_id
181+
WHERE mr.message_id = p_message_id
182+
AND mr.recipient_user_id = p_user_id
183+
AND cp.left_at IS NULL;
184+
185+
IF v_disappear_seconds > 0 AND v_start_on = 'read' THEN
186+
v_expires_at := NOW() + (v_disappear_seconds || ' seconds')::INTERVAL;
187+
ELSE
188+
v_expires_at := NULL;
189+
END IF;
190+
191+
UPDATE public.message_recipients
192+
SET
193+
read_at = NOW(),
194+
expires_at = COALESCE(expires_at, v_expires_at)
195+
WHERE message_id = p_message_id
196+
AND recipient_user_id = p_user_id;
197+
198+
UPDATE public.deliveries
199+
SET read_ts = NOW()
200+
WHERE message_id = p_message_id
201+
AND recipient_user_id = p_user_id;
202+
END;
203+
$function$;
204+
205+
CREATE OR REPLACE FUNCTION public.fn_create_deliveries_for_message(p_message_id uuid)
206+
RETURNS void
207+
LANGUAGE plpgsql
208+
SECURITY DEFINER
209+
SET search_path TO ''
210+
AS $function$
211+
DECLARE
212+
v_conversation_id UUID;
213+
v_sender UUID;
214+
BEGIN
215+
SELECT conversation_id, sender_id INTO v_conversation_id, v_sender
216+
FROM public.messages WHERE id = p_message_id;
217+
218+
IF auth.uid() IS NOT NULL AND NOT EXISTS (
219+
SELECT 1 FROM public.users u
220+
WHERE u.id = v_sender AND u.auth_user_id = auth.uid()
221+
) THEN
222+
RETURN;
223+
END IF;
224+
225+
INSERT INTO public.deliveries (message_id, recipient_user_id, delivered_ts, expires_at)
226+
SELECT
227+
p_message_id,
228+
cp.user_id,
229+
NOW(),
230+
CASE
231+
WHEN cp.disappear_seconds > 0 AND cp.start_on = 'delivered'
232+
THEN NOW() + MAKE_INTERVAL(secs => cp.disappear_seconds)
233+
ELSE NULL
234+
END
235+
FROM public.conversation_participants cp
236+
WHERE cp.conversation_id = v_conversation_id
237+
AND cp.user_id != v_sender
238+
AND cp.left_at IS NULL;
239+
END;
240+
$function$;
241+
242+
REVOKE EXECUTE ON FUNCTION public.fn_cleanup_expired_messages(uuid) FROM PUBLIC, anon, authenticated;
243+
GRANT EXECUTE ON FUNCTION public.fn_cleanup_expired_messages(uuid) TO service_role;
244+
245+
REVOKE EXECUTE ON FUNCTION public.archive_conversation(uuid, uuid) FROM PUBLIC, anon;
246+
GRANT EXECUTE ON FUNCTION public.archive_conversation(uuid, uuid) TO authenticated, service_role;
247+
REVOKE EXECUTE ON FUNCTION public.unarchive_conversation(uuid, uuid) FROM PUBLIC, anon;
248+
GRANT EXECUTE ON FUNCTION public.unarchive_conversation(uuid, uuid) TO authenticated, service_role;
249+
REVOKE EXECUTE ON FUNCTION public.update_user_activity(uuid) FROM PUBLIC, anon;
250+
GRANT EXECUTE ON FUNCTION public.update_user_activity(uuid) TO authenticated, service_role;
251+
REVOKE EXECUTE ON FUNCTION public.fn_get_user_active_deliveries(uuid) FROM PUBLIC, anon;
252+
GRANT EXECUTE ON FUNCTION public.fn_get_user_active_deliveries(uuid) TO authenticated, service_role;
253+
REVOKE EXECUTE ON FUNCTION public.fn_mark_message_read(uuid, uuid) FROM PUBLIC, anon;
254+
GRANT EXECUTE ON FUNCTION public.fn_mark_message_read(uuid, uuid) TO authenticated, service_role;
255+
REVOKE EXECUTE ON FUNCTION public.fn_create_deliveries_for_message(uuid) FROM PUBLIC, anon;
256+
GRANT EXECUTE ON FUNCTION public.fn_create_deliveries_for_message(uuid) TO authenticated, service_role;

0 commit comments

Comments
 (0)