From 4bd1f7bc7ee081b23486d98ceae8b6b6d9d408c5 Mon Sep 17 00:00:00 2001
From: atul-upadhyay-7 <atul-upadhyay-7@users.noreply.github.com>
Date: Wed, 5 Aug 2026 02:21:34 +0530
Subject: [PATCH] fix: make doubt upvotes forge-proof via secure RPC (#1928)

Any authenticated user could set upvotes on any doubt to an arbitrary value
(loose UPDATE policy) and insert doubts with fabricated counts (WITH CHECK
true). Restore the strict INSERT policy (upvotes = 0 + attribution rules),
revoke direct UPDATE from clients, add a one-vote-per-user doubt_upvotes
table, and move upvoting to the SECURITY DEFINER upvote_doubt() RPC. Update
the frontend to call the RPC instead of computing counts client-side.
---
 src/pages/AnonymousDoubts.tsx                 | 13 ++-
 .../20260805000005_secure_doubt_upvotes.sql   | 90 +++++++++++++++++++
 2 files changed, 99 insertions(+), 4 deletions(-)
 create mode 100644 supabase/migrations/20260805000005_secure_doubt_upvotes.sql

diff --git a/src/pages/AnonymousDoubts.tsx b/src/pages/AnonymousDoubts.tsx
index 3cc94816..8de6fea2 100644
--- a/src/pages/AnonymousDoubts.tsx
+++ b/src/pages/AnonymousDoubts.tsx
@@ -72,6 +72,10 @@ export default function AnonymousDoubts() {
 
   const upvote = async (id: string) => {
     if (votedIds.has(id)) return;
+    if (!user) {
+      toast.error("Please log in to upvote a doubt.");
+      return;
+    }
     const doubt = doubts.find((d) => d.id === id);
     if (!doubt) return;
     const newCount = doubt.upvotes + 1;
@@ -79,10 +83,11 @@ export default function AnonymousDoubts() {
     setDoubts(
       doubts.map((d) => (d.id === id ? { ...d, upvotes: newCount } : d))
     );
-    const { error } = await (supabase as any)
-      .from("doubts")
-      .update({ upvotes: newCount })
-      .eq("id", id);
+    // SECURITY (#1928): upvotes are incremented server-side by the
+    // SECURITY DEFINER RPC upvote_doubt() — never computed client-side.
+    const { error } = await (supabase as any).rpc("upvote_doubt", {
+      p_doubt_id: id,
+    });
     if (error) {
       setDoubts(
         doubts.map((d) => (d.id === id ? { ...d, upvotes: doubt.upvotes } : d))
diff --git a/supabase/migrations/20260805000005_secure_doubt_upvotes.sql b/supabase/migrations/20260805000005_secure_doubt_upvotes.sql
new file mode 100644
index 00000000..f0927d0c
--- /dev/null
+++ b/supabase/migrations/20260805000005_secure_doubt_upvotes.sql
@@ -0,0 +1,90 @@
+-- Make doubt upvotes forge-proof
+-- Issue: https://github.com/durdana3105/peer-learning/issues/1928
+--
+-- Problem:
+--   1. The consolidated INSERT policy is WITH CHECK (true), so any
+--      authenticated user can insert a doubt with a fabricated upvotes count
+--      and arbitrary user attribution.
+--   2. The UPDATE policy allows ANY authenticated user to set upvotes on ANY
+--      doubt to any value (only upvotes >= 0 is enforced), so ranking can be
+--      manipulated directly:
+--        update public.doubts set upvotes = 999999 where id = '<any doubt>';
+--
+-- Fix:
+--   1. Restore the strict INSERT policy (upvotes must be 0; attribution must
+--      match the caller: anonymous => user_id IS NULL, named => own user_id).
+--   2. REVOKE UPDATE on doubts from anon + authenticated. Upvoting moves to
+--      the SECURITY DEFINER RPC upvote_doubt() which atomically increments.
+--   3. Add a doubt_upvotes table (unique per user + doubt) so each user can
+--      vote at most once; the RPC enforces it and is idempotent.
+
+-- 1. Strict INSERT policy (matches the original anonymous_doubts design).
+DROP POLICY IF EXISTS "Users can insert doubts" ON public.doubts;
+
+CREATE POLICY "users_can_insert_doubts_strict" ON public.doubts
+  FOR INSERT TO authenticated
+  WITH CHECK (
+    auth.uid() IS NOT NULL
+    AND (
+      (anonymous = true AND user_id IS NULL)
+      OR (anonymous = false AND user_id = auth.uid())
+    )
+    AND upvotes = 0
+  );
+
+-- 2. No client may UPDATE doubts directly anymore.
+DROP POLICY IF EXISTS "authenticated users can update doubt upvotes" ON public.doubts;
+REVOKE UPDATE ON TABLE public.doubts FROM anon, authenticated;
+
+-- 3. One-vote-per-user tracking table.
+CREATE TABLE IF NOT EXISTS public.doubt_upvotes (
+  user_id    uuid not null references auth.users(id) on delete cascade,
+  doubt_id   uuid not null references public.doubts(id) on delete cascade,
+  created_at timestamptz not null default now(),
+  PRIMARY KEY (user_id, doubt_id)
+);
+
+ALTER TABLE public.doubt_upvotes ENABLE ROW LEVEL SECURITY;
+
+CREATE POLICY "users_can_view_own_doubt_votes" ON public.doubt_upvotes
+  FOR SELECT TO authenticated USING (user_id = auth.uid());
+
+CREATE POLICY "users_can_insert_own_doubt_votes" ON public.doubt_upvotes
+  FOR INSERT TO authenticated WITH CHECK (user_id = auth.uid());
+
+-- 4. Atomic, once-per-user upvote RPC.
+CREATE OR REPLACE FUNCTION public.upvote_doubt(p_doubt_id uuid)
+RETURNS void
+LANGUAGE plpgsql
+SECURITY DEFINER
+SET search_path = public
+AS $$
+DECLARE
+  v_uid uuid := auth.uid();
+BEGIN
+  IF v_uid IS NULL THEN
+    RAISE EXCEPTION 'upvote_doubt: authentication required';
+  END IF;
+
+  IF p_doubt_id IS NULL THEN
+    RAISE EXCEPTION 'upvote_doubt: doubt id is required';
+  END IF;
+
+  -- Record the vote first; a duplicate raises unique_violation, in which case
+  -- we exit quietly so repeated clicks are idempotent.
+  BEGIN
+    INSERT INTO public.doubt_upvotes (user_id, doubt_id)
+    VALUES (v_uid, p_doubt_id);
+  EXCEPTION
+    WHEN unique_violation THEN
+      RETURN;
+  END;
+
+  UPDATE public.doubts
+     SET upvotes = upvotes + 1
+   WHERE id = p_doubt_id;
+END;
+$$;
+
+REVOKE ALL ON FUNCTION public.upvote_doubt(uuid) FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION public.upvote_doubt(uuid) TO authenticated;