> On Sep 21, 2020, at 2:09 PM, Robert Haas <robertmh...@gmail.com> wrote:
>
> I think there should be a call to pg_class_aclcheck() here, just like
> the one in pg_prewarm, so that if the superuser does choose to grant
> access, users given access can check tables they anyway have
> permission to access, but not others. Maybe put that in
> check_relation_relkind_and_relam() and rename it. Might want to look
> at the pg_surgery precedent, too.
In the presence of corruption, verify_heapam() reports to the user (in other
words, leaks) metadata about the corrupted rows. Reasoning about the attack
vectors this creates is hard, but a conservative approach is to assume that an
attacker can cause corruption in order to benefit from the leakage, and make
sure the leakage does not violate any reasonable security expectations.
Basing the security decision on whether the user has access to read the table
seems insufficient, as it ignores row level security. Perhaps that is ok if
row level security is not enabled for the table or if the user has been granted
BYPASSRLS. There is another problem, though. There is no grantable privilege
to read dead rows. In the case of corruption, verify_heapam() may well report
metadata about dead rows.
pg_surgery also appears to leak information about dead rows. Owners of tables
can probe whether supplied TIDs refer to dead rows. If a table containing
sensitive information has rows deleted prior to ownership being transferred,
the new owner of the table could probe each page of deleted data to determine
something of the content that was there. Information about the number of
deleted rows is already available through the pg_stat_* views, but those views
don't give such a fine-grained approach to figuring out how large each deleted
row was. For a table with fixed content options, the content can sometimes be
completely inferred from the length of the row. (Consider a table with a
single text column containing either "approved" or "denied".)
But pg_surgery is understood to be a collection of sharp tools only to be used
under fairly exceptional conditions. amcheck, on the other hand, is something
that feels safer and more reasonable to use on a regular basis, perhaps from a
cron job executed by a less trusted user. Forcing the user to be superuser
makes it clearer that this feeling of safety is not justified.
I am inclined to just restrict verify_heapam() to superusers and be done. What
do you think?
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
