two-factor-auth and notifications-manager run with the service role, so they
bypass RLS and whatever user id they act on is the entire authorisation
decision. Both read that id from the request body, and neither checked who was
calling. Access-Control-Allow-Origin was '*' on both.
Confirmed against the live endpoint: with only the public anon key and no user
session, from an arbitrary origin,
POST /functions/v1/two-factor-auth {"action":"check","userId":"<any-uuid>"}
-> 200 {"success":true,"enabled":false}
VERIFY_JWT is on and does not stop this, because the anon key is itself a valid
JWT and every browser has it. The same path reaches "disable", so anyone could
turn off another account's 2FA, and read, delete or forge their notifications.
The client already sends the signed-in user's token — functions.invoke puts the
session access token in the Authorization header — so the caller can simply be
read from it. Both functions now do that and ignore the body's userId. An
anon-key call carries no user, so it is rejected with 401.
create is the one action that may target someone else, because AdminPanel
notifies other users; that path now requires admin or super_admin in
admin_roles. CORS drops to the same allowlist the other functions here use.
No frontend change: the body may keep sending userId, it is simply not read.
Note: 2FA is not yet enforced at login — TwoFactorVerify is not referenced
anywhere and Login.tsx never checks it, so enrolling does not currently protect
sign-in. Tracked separately; this change is about the endpoints themselves.
A bounded adversarial re-verification of the new fund-custody auth found two real
bypasses that the fix rounds missed; both fixed here, plus executable proof.
- BYPASS #1 (CRITICAL, external): p2p_fiat_trades/p2p_fiat_offers kept USING(true)
anon UPDATE, and confirm-payment / admin_resolve_dispute read the fund DESTINATION
(buyer_id), amount and token straight from those rows. An anon attacker could
`UPDATE p2p_fiat_trades SET buyer_id=<attacker>` so the honest seller's correctly-
signed release paid the attacker. Fix: BEFORE UPDATE triggers freeze the fund-routing
columns (offer_id, seller_id, buyer_id, crypto_amount, fiat_amount, price_per_unit,
escrow_locked_amount on trades; seller_id, token, amount_crypto on offers) for any
non-service-role writer — trigger-level, independent of RLS/grants; status/proof/chat
stay open. (Corrects the earlier triage that dismissed these tables as non-custodial.)
- BYPASS #2 (double-release TOCTOU): confirm-payment checked status then released
non-atomically, so two concurrent differently-nonced valid requests could both release
and drain other buyers' escrow. Fix: atomic compare-and-swap of payment_sent->completed
BEFORE the escrow move (loser gets 409; revert on release failure).
- Hardening: REVOKE EXECUTE on the dead legacy resolve_p2p_dispute mover from
PUBLIC/anon/authenticated.
- Proof: 34 executable tests (Deno) — identity-auth unit tests (sr25519 sign/verify,
challenge money-param binding, citizen/visa ownership, nonce single-use+concurrency,
freshness) + per-handler authorization-flow tests (bad-sig/cross-id/wrong-state/replay,
CAS double-release). All pass.
Round 2 — fixes the CRITICAL discovered during round 1 (as severe as the withdrawal
BOLA): lock_/release_/refund_escrow_internal had default PUBLIC EXECUTE and the client
called release_escrow_internal directly with the anon key, so anyone could drain any
victim's LOCKED balance.
- release_escrow_internal now reachable only via new signed confirm-payment edge
function: verifies seller wallet signature (raw + <Bytes>), asserts the wallet owns
the seller identity, derived user_id == trade.seller_id, trade == payment_sent,
single-use nonce, then release runs with the service role. confirmPaymentReceived
now invokes confirm-payment (no more anon rpc).
- lock_escrow_internal now behind new signed lock-escrow edge function (a caller can
only lock its own balance; stops griefing a victim's balance). Removed a zero-amount
no-op lock call.
- Migration 20260725030000: REVOKE EXECUTE on lock_/release_/refund_escrow_internal
from PUBLIC/anon/authenticated; GRANT to service_role only. refund has no direct
client caller (service-role + admin_resolve_dispute only).
- DEPLOY_RUNBOOK.md consolidates the ordered migrations, edge functions and secrets
for both security rounds. Migration 20260725030000 must ship WITH the two new edge
functions + frontend or offer-create/payment-release break.
Remaining (non-fund, Round 2+ follow-up): read RPCs still key on a non-secret user_id
(binding to a signed session would force a sign-prompt on every passive balance read —
deferred); p2p_fiat_offers/trades/messages retain USING(true) (status labels move no
funds now that all escrow movement is service-role-gated).
Critical/high audit remediation on the custodial P2P ledger. Auth model is
wallet-based; identity (citizen/visa) is cryptographically bound to a wallet
(People Chain tiki.citizenNft / active p2p_visa), which enables a correct fix.
- Withdrawal BOLA (CRITICAL): process-withdraw now requires a wallet-SIGNED
challenge; server verifies signature (raw + <Bytes> forms), asserts the signer
OWNS the identity, consumes a single-use nonce (replay), and derives user_id
server-side — the client-supplied user_id is ignored. process-withdrawal (batch)
now requires the service-role key (was: any bearer, incl. public anon key).
request_withdraw is REVOKEd from anon/authenticated + service-role guarded.
- Financial RLS (HIGH): drop the blanket USING(true) SELECT on user_internal_balances,
p2p_balance_transactions, p2p_deposit_withdraw_requests; lock p2p_user_payment_methods
(IBAN PII) + p2p_fiat_disputes UPDATE to service_role; legitimate reads move behind
scoped SECURITY DEFINER RPCs.
- Deposit integrity: verify-deposit now binds the on-chain sender to identity ownership
before crediting.
- Admin dispute (CRITICAL fund-logic): DisputeResolutionPanel relabeled trades without
moving escrow. New admin-signed resolve-dispute function + admin_resolve_dispute RPC
moves escrow (release/refund/split) atomically with correct accounting (avoids the
double-count in the legacy resolve_p2p_dispute). Client isAdmin documented as cosmetic.
DEPLOY RUNBOOK (gated; owner runs): 1) apply migrations 20260225/20260725* in order;
2) deploy edge functions process-withdraw, process-withdrawal, verify-deposit, resolve-dispute;
3) set edge secrets PEOPLE_RPC_ENDPOINT (+ optional ADMIN_WALLETS); 4) ship frontend.
Migrations + functions + frontend must go together or the app breaks.
KNOWN RESIDUAL (Round 2 — as severe as the withdrawal BOLA): release_/lock_/refund_
escrow_internal still have PUBLIC EXECUTE and the client calls release_escrow_internal
directly with the anon key from confirmPaymentReceived -> an anon caller can drain any
victim's LOCKED balance. Fix = a wallet-signed confirm-payment edge function (same
pattern as withdrawals) before revoking PUBLIC execute. Not yet fixed.
Satoshi Qazi Muhammed