RLS bypass: ON CONFLICT DO UPDATE/SELECT evaluates WHERE before RLS check

Hi,

While testing ON CONFLICT DO SELECT on master, I found that the
user's WHERE clause is evaluated against the existing row before the
row is checked against the RLS policies. A user who cannot see a row
through SELECT/UPDATE policies can therefore read its columns, as long
as a proposed row conflicts with it on a unique key.

Both ExecOnConflictUpdate() and ExecOnConflictSelect() in
nodeModifyTable.c do ExecQual(onConflict*Where) first and only then
ExecWithCheckOptions(WCO_RLS_CONFLICT_CHECK). The CREATE POLICY docs
say the existing row is checked against the USING expressions first.

Repro (attached repro.sql, as superuser; then as a role that can't see
row 2):

insert into oc values (2,'u_sel','x')
on conflict (id) do select where leak(oc.secret) returning id;
NOTICE: LEAKED: HIDDEN-VALUE

where leak() is a plpgsql function that does RAISE NOTICE. The same
works with DO UPDATE. Without any function, a plain predicate is an
oracle: "where oc.secret like 'HID%'" raises the RLS error when it
matches and is a silent no-op when it doesn't. The attacker needs to
hit the hidden row's key, which is easy to enumerate for serial ids.

DO UPDATE has had this ordering since 168d5805e4 (2015); DO SELECT
(88327092ff) copied it. I have only tested master and 16beta2, I haven't
tested the other stable branches.

The attached patch swaps the order in both functions, so RLS is checked
before the WHERE clause. With it the repro no longer leaks, and the
regress suite passes. No existing test covers this, so it also needs
a regression test. One behavior change: a row failing the RLS check
now raises the error even when WHERE would have been false. That
matches what the docs describe.

Thanks,
João Marcelo