pgsql: Restrict accesses to non-system views and foreign tables during
Restrict accesses to non-system views and foreign tables during pg_dump.
When pg_dump retrieves the list of database objects and performs the
data dump, there was possibility that objects are replaced with others
of the same name, such as views, and access them. This vulnerability
could result in code execution with superuser privileges during the
pg_dump process.
This issue can arise when dumping data of sequences, foreign
tables (only 13 or later), or tables registered with a WHERE clause in
the extension configuration table.
To address this, pg_dump now utilizes the newly introduced
restrict_nonsystem_relation_kind GUC parameter to restrict the
accesses to non-system views and foreign tables during the dump
process. This new GUC parameter is added to back branches too, but
these changes do not require cluster recreation.
Back-patch to all supported branches.
Reviewed-by: Noah Misch
Security: CVE-2024-7348
Backpatch-through: 12
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/66e94448abec3aad04faf0a79cab4881ae08e08a
Modified Files
--------------
contrib/postgres_fdw/expected/postgres_fdw.out | 11 +++++
contrib/postgres_fdw/sql/postgres_fdw.sql | 8 ++++
doc/src/sgml/config.sgml | 17 +++++++
doc/src/sgml/ref/pg_dump.sgml | 8 ++++
src/backend/foreign/foreign.c | 10 ++++
src/backend/optimizer/plan/createplan.c | 13 ++++++
src/backend/optimizer/util/plancat.c | 12 +++++
src/backend/rewrite/rewriteHandler.c | 17 +++++++
src/backend/tcop/postgres.c | 64 ++++++++++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 12 +++++
src/bin/pg_dump/pg_dump.c | 47 +++++++++++++++++++
src/include/tcop/tcopprot.h | 6 +++
src/include/utils/guc_hooks.h | 3 ++
src/test/regress/expected/create_view.out | 19 +++++++-
src/test/regress/sql/create_view.sql | 9 ++++
15 files changed, 255 insertions(+), 1 deletion(-)
On 05.08.24 15:07, Masahiko Sawada wrote:
To address this, pg_dump now utilizes the newly introduced
restrict_nonsystem_relation_kind GUC parameter to restrict the
accesses to non-system views and foreign tables during the dump
process. This new GUC parameter is added to back branches too, but
these changes do not require cluster recreation.
This documentation of this new parameter is a bit hard to understand.
The description in guc_tables.c is
"Sets relation kinds of non-system relation to restrict use"
which is hard to understand even knowing what this setting is supposed
to do.
In config.sgml it says
+ This variable specifies relation kind to which access is restricted.
+ It contains a comma-separated list of relation kind. Currently, the
+ supported relation kinds are <literal>view</literal> and
+ <literal>foreign-table</literal>.This does not mention "system" or "non-system" at all.
Also, the phrase "to which access is restricted" can be interpreted in
two opposite ways:
- access to those relations is prohibited
- access is limited to those relations
Also nothing anywhere clarifies what "restricted" means here, and the
term introduces unnecessary ambiguity.
Can we come up with some more precise and easier-to-understand language?
On Mon, Aug 26, 2024 at 7:14 AM Peter Eisentraut <peter@eisentraut.org> wrote:
On 05.08.24 15:07, Masahiko Sawada wrote:
To address this, pg_dump now utilizes the newly introduced
restrict_nonsystem_relation_kind GUC parameter to restrict the
accesses to non-system views and foreign tables during the dump
process. This new GUC parameter is added to back branches too, but
these changes do not require cluster recreation.This documentation of this new parameter is a bit hard to understand.
The description in guc_tables.c is"Sets relation kinds of non-system relation to restrict use"
which is hard to understand even knowing what this setting is supposed
to do.In config.sgml it says
+ This variable specifies relation kind to which access is restricted. + It contains a comma-separated list of relation kind. Currently, the + supported relation kinds are <literal>view</literal> and + <literal>foreign-table</literal>.This does not mention "system" or "non-system" at all.
Also, the phrase "to which access is restricted" can be interpreted in
two opposite ways:- access to those relations is prohibited
- access is limited to those relationsAlso nothing anywhere clarifies what "restricted" means here, and the
term introduces unnecessary ambiguity.Can we come up with some more precise and easier-to-understand language?
Maybe using the word "prohibit" makes it a bit clear? For example,
In guc_tables.c:
Prohibits access to non-system relations of specified kinds
In doc:
Set relation kinds of non-system relations to which access is
prohibited. It takes a comma-separated list of relation kinds.
Currently, the supported relation kinds are view and foreign-table.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
On 26.08.24 21:05, Masahiko Sawada wrote:
Maybe using the word "prohibit" makes it a bit clear? For example,
I think that would be much clearer, yes.
In guc_tables.c:
Prohibits access to non-system relations of specified kinds
End with period.
In doc:
Set relation kinds of non-system relations to which access is
prohibited.
Still a bit awkward. Maybe "Set relation kinds for which access to
non-system relations is prohibited."
It takes a comma-separated list of relation kinds.
Currently, the supported relation kinds are view and foreign-table.
Maybe in the documentation it would also be appropriate to mention that
this is meant to be used by pg_dump, not for general use -- unless it is?
Peter Eisentraut <peter@eisentraut.org> writes:
Maybe in the documentation it would also be appropriate to mention that
this is meant to be used by pg_dump, not for general use -- unless it is?
I'd vote against that. I think other catalog-scanning tools might
like to use this too.
regards, tom lane
On Tue, Aug 27, 2024 at 6:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Maybe in the documentation it would also be appropriate to mention that
this is meant to be used by pg_dump, not for general use -- unless it is?I'd vote against that. I think other catalog-scanning tools might
like to use this too.
I've attached the patch to improve the descriptions while leaving this part.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
improve_description.patchapplication/octet-stream; name=improve_description.patchDownload+4-4
On 27.08.24 22:02, Masahiko Sawada wrote:
On Tue, Aug 27, 2024 at 6:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Maybe in the documentation it would also be appropriate to mention that
this is meant to be used by pg_dump, not for general use -- unless it is?I'd vote against that. I think other catalog-scanning tools might
like to use this too.I've attached the patch to improve the descriptions while leaving this part.
Thanks, I find this clearer.
On Thu, Aug 29, 2024 at 7:29 AM Peter Eisentraut <peter@eisentraut.org> wrote:
On 27.08.24 22:02, Masahiko Sawada wrote:
On Tue, Aug 27, 2024 at 6:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Maybe in the documentation it would also be appropriate to mention that
this is meant to be used by pg_dump, not for general use -- unless it is?I'd vote against that. I think other catalog-scanning tools might
like to use this too.I've attached the patch to improve the descriptions while leaving this part.
Thanks, I find this clearer.
Thank you for checking. I'm going to push it for all branches unless
there are other comments.
BTW I'd like to revisit to improve the error message for the new GUC
parameter. I've drafted the patch.We have the check in three places
and the check in GetFdwRoutine() covers the TRUNCATE command case. The
patch adds the check to ExecuteTruncateGuts() and adds the relation
name to the all log messages for that check. I'm slightly concerned
about having the check in more places, so get feedback.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com