Re: BUG #19523: psql tab-completion shadows pg_db_role_setting

Started by Vismay Tiwari2 months ago6 messagesbugs
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

This thread has been committed, so CI has stopped here. Anything below is the last result it produced.

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253032
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 11, 2026 at 05:52 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253032_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253032_1 && git checkout t253032_1

Patchset v1 (message #1) is on t253032_1

Jump to latest
#1Vismay Tiwari
vismay.t@gmail.com

Hi,

Reproduced on current master. The tab-completion query for
"ALTER DATABASE ... RESET" (Query_for_list_of_database_vars) references
pg_db_role_setting and pg_database without a pg_catalog qualification, so a
same-named table earlier in search_path shadows the catalog:

CREATE SCHEMA attacker;
CREATE TABLE attacker.pg_db_role_setting (setdatabase oid, setrole
oid, setconfig text[]);
INSERT INTO attacker.pg_db_role_setting
SELECT oid, 0, ARRAY['evil_var=x'] FROM pg_database WHERE
datname = 'postgres';
SET search_path = attacker, pg_catalog;
-- "ALTER DATABASE postgres RESET <TAB>" then offers evil_var

The attached patch qualifies both catalogs with pg_catalog, matching the
qualification already used for unnest()/split_part() in the same query and for
the catalogs in the analogous subscription-variable completion query.

No test is included -- tab-completion query internals aren't covered by the
TAP suite, and this follows the earlier qualification fixes in the same area.
Affects v18 and master (the query was added in v18).

Regards,
Vismay Tiwari

Attachments:

t253032_1
v1-0001-psql-schema-qualify-catalog-references-in-a-tab-c.patchapplication/octet-stream; name=v1-0001-psql-schema-qualify-catalog-references-in-a-tab-c.patchDownload+2-3
#2Kirill Reshke
reshkekirill@gmail.com
In reply to: Vismay Tiwari (#1)

Hi! You seem to create two threads on the issue, so I don't quite
understand where to respond. anyway:

On Thu, 9 Jul 2026, 15:14 Vismay Tiwari, <vismay.t@gmail.com> wrote:

Hi,

Reproduced on current master. The tab-completion query for
"ALTER DATABASE ... RESET" (Query_for_list_of_database_vars) references
pg_db_role_setting and pg_database without a pg_catalog qualification, so a
same-named table earlier in search_path shadows the catalog:

CREATE SCHEMA attacker;
CREATE TABLE attacker.pg_db_role_setting (setdatabase oid, setrole
oid, setconfig text[]);
INSERT INTO attacker.pg_db_role_setting
SELECT oid, 0, ARRAY['evil_var=x'] FROM pg_database WHERE
datname = 'postgres';
SET search_path = attacker, pg_catalog;
-- "ALTER DATABASE postgres RESET <TAB>" then offers evil_var

Preventing this makes sense in case somebody accidentally misconfigured
their server. Which is not very probable for a table with `
pg_db_role_setting` name.
Also note that this is not a vulnerability: from
https://www.postgresql.org/docs/current/app-psql.html says:

If untrusted users have access to a database that has not adopted a secure
schema usage pattern, begin your session by removing publicly-writable
schemas from search_path.

Anyway +1 on fixing

#3Vismay Tiwari
vismay.t@gmail.com
In reply to: Kirill Reshke (#2)

Hi Kirill,

Thanks, and sorry for the two threads — the second was an accidental
resend that didn't thread properly on my end , so let's keep it here.

You're right that it's not a vulnerability, and the "attacker" framing
was overblown — the docs are clear that removing publicly-writable
schemas from search_path is the real safeguard. The intent is just
consistency and robustness: the same query already qualifies
unnest()/split_part() with pg_catalog, and the analogous
subscription-variable completion query qualifies its catalogs, so this
only brings pg_db_role_setting/pg_database in line. As a side benefit
it avoids a surprising tab-completion result for anyone who happens to
have a same-named table earlier in search_path, misconfigured or not.

Thanks for the +1.

Regards,
Vismay

Show quoted text

On Fri, Jul 10, 2026 at 10:19 AM Kirill Reshke <reshkekirill@gmail.com> wrote:

Hi! You seem to create two threads on the issue, so I don't quite understand where to respond. anyway:

On Thu, 9 Jul 2026, 15:14 Vismay Tiwari, <vismay.t@gmail.com> wrote:

Hi,

Reproduced on current master. The tab-completion query for
"ALTER DATABASE ... RESET" (Query_for_list_of_database_vars) references
pg_db_role_setting and pg_database without a pg_catalog qualification, so a
same-named table earlier in search_path shadows the catalog:

CREATE SCHEMA attacker;
CREATE TABLE attacker.pg_db_role_setting (setdatabase oid, setrole
oid, setconfig text[]);
INSERT INTO attacker.pg_db_role_setting
SELECT oid, 0, ARRAY['evil_var=x'] FROM pg_database WHERE
datname = 'postgres';
SET search_path = attacker, pg_catalog;
-- "ALTER DATABASE postgres RESET <TAB>" then offers evil_var

Preventing this makes sense in case somebody accidentally misconfigured their server. Which is not very probable for a table with `pg_db_role_setting` name.
Also note that this is not a vulnerability: from https://www.postgresql.org/docs/current/app-psql.html says:

If untrusted users have access to a database that has not adopted a secure
schema usage pattern, begin your session by removing publicly-writable
schemas from search_path.

Anyway +1 on fixing

#4Paul Kim
mok03127@gmail.com
In reply to: Vismay Tiwari (#3)

Hi Vismay,

I had a look at the v1 patch, looks good to me.

Query_for_list_of_database_vars really is the only completion query still
referencing catalogs unqualified -- split_part()/unnest() right next to it
are already pg_catalog-qualified, and the sibling queries
(Query_for_list_of_databases, _tablespaces, ...) all qualify theirs too, so
this just brings it in line.

I reproduced it on master with a shadowing table:

CREATE SCHEMA s;
CREATE TABLE s.pg_db_role_setting (setdatabase oid, setrole oid, setconfig text[]);
INSERT INTO s.pg_db_role_setting
SELECT oid, 0, ARRAY['evil_var=x'] FROM pg_catalog.pg_database
WHERE datname = 'postgres';
SET search_path = s, pg_catalog;

Before the patch the query hands back 'evil_var'; after qualifying both
catalogs it returns nothing, and a real ALTER DATABASE ... SET still shows up
fine. Applies and builds cleanly here.

On back-patching: the macro came in with v18 (9df8727c50), it's in
REL_18_STABLE and master but not REL_17, so v18 + master matches what you
said.

Agreed on leaving out a test -- the completion queries aren't covered by TAP
anyway.

One small nit, and it's not really about the code: since we've settled that
this is more of a consistency/robustness thing than a security issue, the
commit message's "feeds arbitrary values" wording could probably be toned
down, but that's the committer's call.

Looks ready for committer to me.

Regards,
Paul

#5Fujii Masao
masao.fujii@gmail.com
In reply to: Paul Kim (#4)

On Fri, Jul 17, 2026 at 11:28 PM Paul Kim <mok03127@gmail.com> wrote:

Looks ready for committer to me.

The patch looks good to me. Barring any objections, I'll commit it.

BTW, commit dbf5a83d465 schema-qualified the unnest() call in this same
query, but left the catalog references unqualified. This makes me wonder
if this issue could be considered an oversight in that commit.

Regards,

--
Fujii Masao

#6Fujii Masao
masao.fujii@gmail.com
In reply to: Fujii Masao (#5)

On Fri, Sep 11, 2026 at 12:33 AM Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Jul 17, 2026 at 11:28 PM Paul Kim <mok03127@gmail.com> wrote:

Looks ready for committer to me.

The patch looks good to me. Barring any objections, I'll commit it.

I've pushed the patch. Thanks!

Regards,

--
Fujii Masao