Skip to content

transactions RLS restricts rows but not columns — a student can self-insert a 'successful' sale #528

Description

@guillermoscript

Found while reviewing #525 / working #512 (2026-07-25). Verified against a local database.

Problem

transactions grants ALL to authenticated:

-- 20260126190500_lms_complete.sql:3692
GRANT ALL ON TABLE "public"."transactions" TO "authenticated";

and its RLS policies restrict which rows a user may write, never which columns:

-- 20260313025318_rls_transactions.sql
CREATE POLICY "Users can create own transactions"
  ON public.transactions FOR INSERT TO authenticated
  WITH CHECK (auth.uid() = user_id AND tenant_id = get_tenant_id());

CREATE POLICY "Users can update own transactions"
  ON public.transactions FOR UPDATE TO authenticated
  USING (auth.uid() = user_id AND tenant_id = get_tenant_id())
  WITH CHECK (auth.uid() = user_id AND tenant_id = get_tenant_id());

So an authenticated student can POST /rest/v1/transactions (or PATCH a row they already own) with any column values they like, as long as user_id is theirs and tenant_id is their current tenant. status, amount, product_id, plan_id, payment_provider and currency are all in scope.

Impact

Two separate consequences, both reached without touching a payment provider:

  1. Free enrolment. trigger_manage_transactions (AFTER INSERT/UPDATE) calls enroll_user(NEW.user_id, NEW.product_id) whenever product_id is set and status = 'successful', and handle_new_subscription(...) on the plan_id branch. A self-inserted row with status: 'successful' therefore grants entitlements — i.e. paid course access — with no money involved.
  2. Fabricated payout liability. getPayoutsOwed() reads transactions where status IN ('successful','refunded') and payment_provider IN (paypal, lemonsqueezy, binance). A self-inserted row carrying one of those providers lands in grossOwed, so the platform's own payout dashboard reports it owes a school money for a sale that never happened.

Scope note

#512 (PR #527) closes the school_percentage_snapshot column specifically, by making the database compute and freeze that value regardless of what the caller sends. That is one column. The rest of the row is still caller-controlled, which is why this is filed separately rather than folded in.

Possible directions

Not a recommendation yet — these need weighing against the paths that legitimately insert transactions on the user-scoped client (app/api/payments/checkout/route.ts, app/[locale]/(public)/checkout/actions.ts):

  • Column-level privileges: revoke table-level INSERT, UPDATE from authenticated and re-GRANT an explicit column list. Note a bare REVOKE ... (column) is a no-op while the table-level grant is held. The risk is silently breaking any column omitted from the new list.
  • Tighten the policies to pin the columns that matter — e.g. WITH CHECK (... AND status = 'pending' AND payment_provider IS NULL) on INSERT, so only a webhook or a service-role path can produce a successful row.
  • Move transaction creation behind a SECURITY DEFINER RPC and drop the direct grant entirely.

The second is probably the smallest change that closes the enrolment path; worth confirming no legitimate caller inserts a non-pending transaction on the user-scoped client first.

Metadata

Metadata

Labels

bugSomething isn't workingpaymentsPayment provider / billing relatedsecuritySecurity vulnerability or hardeningseverity:highHigh severity security finding

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions