Make inherited TRUNCATE perform access permission checks on parent table only.

Previously, TRUNCATE command through a parent table checked the
permissions on not only the parent table but also the children tables
inherited from it. This was a bug and inherited queries should perform
access permission checks on the parent table only. This commit fixes
that bug.

Back-patch to all supported branches.

Author: Amit Langote
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAHGQGwFHdSvifhJE+-GSNqUHSfbiKxaeQQ7HGcYz6SC2n_oDcg@mail.gmail.com

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 38386fb9cf940eac876a625a06481f9960ddd9f2..9b0dc3fd10dec11b9bca6d1833472aed82bdd818 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -300,7 +300,9 @@ struct DropRelationCallbackState
 #define child_dependency_type(child_is_partition)  \
    ((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
 
-static void truncate_check_rel(Relation rel);
+static void truncate_check_rel(Oid relid, Form_pg_class reltuple);
+static void truncate_check_perms(Oid relid, Form_pg_class reltuple);
+static void truncate_check_activity(Relation rel);
 static List *MergeAttributes(List *schema, List *supers, char relpersistence,
                bool is_partition, List **supOids, List **supconstr,
                int *supOidCount);
@@ -1381,7 +1383,11 @@ ExecuteTruncate(TruncateStmt *stmt)
            heap_close(rel, lockmode);
            continue;
        }
-       truncate_check_rel(rel);
+
+       truncate_check_rel(myrelid, rel->rd_rel);
+       truncate_check_perms(myrelid, rel->rd_rel);
+       truncate_check_activity(rel);
+
        rels = lappend(rels, rel);
        relids = lappend_oid(relids, myrelid);
        /* Log this relation only if needed for logical decoding */
@@ -1420,7 +1426,15 @@ ExecuteTruncate(TruncateStmt *stmt)
                    continue;
                }
 
-               truncate_check_rel(rel);
+               /*
+                * Inherited TRUNCATE commands perform access
+                * permission checks on the parent table only.
+                * So we skip checking the children's permissions
+                * and don't call truncate_check_perms() here.
+                */
+               truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);
+               truncate_check_activity(rel);
+
                rels = lappend(rels, rel);
                relids = lappend_oid(relids, childrelid);
                /* Log this relation only if needed for logical decoding */
@@ -1503,7 +1517,9 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
                ereport(NOTICE,
                        (errmsg("truncate cascades to table \"%s\"",
                                RelationGetRelationName(rel))));
-               truncate_check_rel(rel);
+               truncate_check_rel(relid, rel->rd_rel);
+               truncate_check_perms(relid, rel->rd_rel);
+               truncate_check_activity(rel);
                rels = lappend(rels, rel);
                relids = lappend_oid(relids, relid);
                /* Log this relation only if needed for logical decoding */
@@ -1759,35 +1775,51 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
  * Check that a given rel is safe to truncate.  Subroutine for ExecuteTruncate
  */
 static void
-truncate_check_rel(Relation rel)
+truncate_check_rel(Oid relid, Form_pg_class reltuple)
 {
-   AclResult   aclresult;
+   char       *relname = NameStr(reltuple->relname);
 
    /*
     * Only allow truncate on regular tables and partitioned tables (although,
     * the latter are only being included here for the following checks; no
     * physical truncation will occur in their case.)
     */
-   if (rel->rd_rel->relkind != RELKIND_RELATION &&
-       rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+   if (reltuple->relkind != RELKIND_RELATION &&
+       reltuple->relkind != RELKIND_PARTITIONED_TABLE)
        ereport(ERROR,
                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                errmsg("\"%s\" is not a table",
-                       RelationGetRelationName(rel))));
-
-   /* Permissions checks */
-   aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
-                                 ACL_TRUNCATE);
-   if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
-                      RelationGetRelationName(rel));
+                errmsg("\"%s\" is not a table", relname)));
 
-   if (!allowSystemTableMods && IsSystemRelation(rel))
+   if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 errmsg("permission denied: \"%s\" is a system catalog",
-                       RelationGetRelationName(rel))));
+                       relname)));
+}
+
+/*
+ * Check that current user has the permission to truncate given relation.
+ */
+static void
+truncate_check_perms(Oid relid, Form_pg_class reltuple)
+{
+   char       *relname = NameStr(reltuple->relname);
+   AclResult   aclresult;
 
+   /* Permissions checks */
+   aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
+   if (aclresult != ACLCHECK_OK)
+       aclcheck_error(aclresult, get_relkind_objtype(reltuple->relkind),
+                      relname);
+}
+
+/*
+ * Set of extra sanity checks to check if a given relation is safe to
+ * truncate.
+ */
+static void
+truncate_check_activity(Relation rel)
+{
    /*
     * Don't allow truncate on temp tables of other backends ... their local
     * buffer manager is not going to cope.

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index a8346e1717438d4cf630c0fe07f88f14a12f7e90..6221601c5d4a0e21aba4d35029e8349da6214df8 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -695,6 +695,27 @@ SELECT oid FROM atestp2; -- ok
 -----
 (0 rows)
 
+-- child's permissions do not apply when operating on parent
+SET SESSION AUTHORIZATION regress_priv_user1;
+REVOKE ALL ON atestc FROM regress_priv_user2;
+GRANT ALL ON atestp1 TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_user2;
+SELECT f2 FROM atestp1; -- ok
+ f2 
+----
+(0 rows)
+
+SELECT f2 FROM atestc; -- fail
+ERROR:  permission denied for table atestc
+DELETE FROM atestp1; -- ok
+DELETE FROM atestc; -- fail
+ERROR:  permission denied for table atestc
+UPDATE atestp1 SET f1 = 1; -- ok
+UPDATE atestc SET f1 = 1; -- fail
+ERROR:  permission denied for table atestc
+TRUNCATE atestp1; -- ok
+TRUNCATE atestc; -- fail
+ERROR:  permission denied for table atestc
 -- privileges on functions, languages
 -- switch to superuser
 \c -

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index c1e42d1be2fe6a7fd0d26389c8b1f50ab058f243..c25157b32d8d4b380a062d2de6f5d49a3392d4ec 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
 SELECT atestp2 FROM atestp2; -- ok
 SELECT oid FROM atestp2; -- ok
 
+-- child's permissions do not apply when operating on parent
+SET SESSION AUTHORIZATION regress_priv_user1;
+REVOKE ALL ON atestc FROM regress_priv_user2;
+GRANT ALL ON atestp1 TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_user2;
+SELECT f2 FROM atestp1; -- ok
+SELECT f2 FROM atestc; -- fail
+DELETE FROM atestp1; -- ok
+DELETE FROM atestc; -- fail
+UPDATE atestp1 SET f1 = 1; -- ok
+UPDATE atestc SET f1 = 1; -- fail
+TRUNCATE atestp1; -- ok
+TRUNCATE atestc; -- fail
+
 -- privileges on functions, languages
 
 -- switch to superuser