pgsql: Make pg_dump's ACL, sec label, and comment entries reliably iden

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Make pg_dump's ACL, sec label, and comment entries reliably iden
Date: 2018-01-22 17:06:48
Message-ID: E1edfYW-0000QH-1l@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Make pg_dump's ACL, sec label, and comment entries reliably identifiable.

_tocEntryRequired() expects that it can identify ACL, SECURITY LABEL,
and COMMENT TOC entries that are for large objects by seeing whether
the tag for them starts with "LARGE OBJECT ". While that works fine
for actual large objects, which are indeed tagged that way, it's
subject to false positives unless every such entry's tag starts with an
appropriate type ID. And in fact it does not work for ACLs, because
up to now we customarily tagged those entries with just the bare name
of the object. This means that an ACL for an object named
"LARGE OBJECT something" would be misclassified as data not schema,
with undesirable results in a schema-only or data-only dump ---
although pg_upgrade seems unaffected, due to the special case for
binary-upgrade mode further down in _tocEntryRequired().

We can fix this by changing all the dumpACL calls to use the label
strings already in use for comments and security labels, which do
follow the convention of starting with an object type indicator.

Well, mostly they follow it. dumpDatabase() got it wrong, using
just the bare database name for those purposes, so that a database
named "LARGE OBJECT something" would similarly be subject to having
its comment or security label dropped or included when not wanted.
Bring that into line too. (Note that up to now, database ACLs have
not been processed by pg_dump, so that this issue doesn't affect them.)

_tocEntryRequired() itself is not free of fault: it was overly liberal
about matching object tags to "LARGE OBJECT " in binary-upgrade mode.
This looks like it is probably harmless because there would be no data
component to strip anyway in that mode, but at best it's trouble
waiting to happen, so tighten that up too.

The possible misclassification of SECURITY LABEL entries for databases is
in principle a security problem, but the opportunities for actual exploits
seem too narrow to be interesting. The other cases seem like just bugs,
since an object owner can change its ACL or comment for himself, he needn't
try to trick someone else into doing it by choosing a strange name.

This has been broken since per-large-object TOC entries were introduced
in 9.0, so back-patch to all supported branches.

Discussion: https://postgr.es/m/21714.1516553459@sss.pgh.pa.us

Branch
------
REL9_6_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/52cc1b484e52faad20a4a6094b015080b3ff8e07

Modified Files
--------------
src/bin/pg_dump/pg_backup_archiver.c | 10 ++++-
src/bin/pg_dump/pg_dump.c | 78 +++++++++++++++++++++---------------
2 files changed, 53 insertions(+), 35 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Eisentraut 2018-01-22 17:11:30 pgsql: PL/Python: Fix tests for older Python versions
Previous Message Tom Lane 2018-01-22 15:01:50 Re: pgsql: Add parallel-aware hash joins.