Proposal: detect read policies whose USING is unconditionally true for unauthenticated sessions (inverted auth.* IS NULL disjunct)

Đang mở
  • Có gì không ổn?
#165 3 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức phù hợp với người mới
45/100
Loại issue
Tính năng
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Ít trao đổi
Công nghệ
postgres
Lĩnh vực
databases, security

Hướng nghiên cứu

Bắt đầu với skill /new-lint và lint hiện có 0024_rls_policy_always_true, sau đó xem lại hợp đồng lint tiêu chuẩn và bố cục kiểm thử của pg_regress. Xác nhận số lint mới và lựa chọn WARN/INFO trước khi tiếp tục. Hoàn tất khi true positive được đề xuất được phát hiện và các trường hợp âm được liệt kê—current_user, IS NULL được điều kiện hóa bằng AND, một predicate có phạm vi giới hạn và current_setting(name, false)—vẫn không bị phát hiện trong các kiểm thử hồi quy.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

Hi! A suggestion for a new Security lint, related to but framed differently from PR #28 ("feat: add lint to detect RLS policies that allow access to anonymous users").

The gap

0024_rls_policy_always_true deliberately scopes its always-true USING check to command in ('UPDATE','DELETE','ALL') and matches only the literal forms ('true','(true)','1=1','(1=1)') — with the comment that SELECT … USING (true) is often intentional. That's the right call for literal true. But a very common real-world shape slips through entirely:

create policy "read own"
  on public.documents for select to authenticated
  using ( auth.uid() is null or owner_id = auth.uid() );

For any request where auth.uid() is NULL (an anonymous session, or any token-less call), auth.uid() is null is true, so the whole USING is true and every row is readable. It isn't literal-true, and it's a SELECT, so 0024 can't see it — yet it exposes the table.

Why this isn't the PR #28 discussion

This lint does not flag "the policy is reachable by anonymous users" — that's a deliberate design choice and not inherently a bug (which, as I read it, was the substance of the pushback on PR #28). It flags a predicate whose top-level OR short-circuits to true for any session where the auth function returns NULL — a logic error that is wrong whether or not anonymous sign-ins are enabled. (Which sessions those are depends on the policy's roles: for a public/anon policy it's any token-less request; for a TO authenticated policy it's the narrower case of an authenticated token with a NULL sub.) An … IS NULL OR … disjunct on an auth function almost always means the author intended the negation (IS NOT NULL AND …) or a scoped check, and accidentally turned the policy into a no-op. Framing it as a correctness defect (rather than an anonymous-access judgement) keeps it squarely in advisor territory.

Proposed detection

A pure SQL view over pg_policies, standard lint contract. It fires on a predicate shape, not on any project setting, so it needs no conditional execution:

  • permissive policy, cmd in ('SELECT','ALL'), granted to anon / authenticated / public;
  • normalized qual contains a top-level OR disjunct of the form <auth fn>() is null, where <auth fn> is genuinely nullable — auth.uid() / auth.jwt() / auth.role(), or current_setting('…', true).
  • Exclusions, to stay false-positive-clean: never-NULL identity functions (current_user / session_user / current_role / user) — those disjuncts are constant-false dead branches, not leaks; and current_setting(name) / current_setting(name, false), which raise rather than return NULL.
  • Severity: WARN/INFO; remediation: "remove the IS NULL disjunct, or replace it with an explicit deny."

A couple of questions before a PR

  1. Would you prefer a fresh lint number, or to revive/rework PR #28?
  2. Severity preference — WARN or INFO?

Happy to implement it with the /new-lint skill + pg_regress tests covering the true positive above plus negatives (current_user is null disjunct, AND-gated is null, a plain scoped predicate, current_setting(name, false) is null).

Ngôn ngữ chính
PLpgSQL
Star
284
Fork
110
Không có pull request nào được merge trong 30 ngày

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của supabase/splinter

Tất cả issue của supabase/splinter

Issue tương tự

Thêm issue về Databases

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.