Make inherited TRUNCATE perform access permission checks on parent table only.
authorFujii Masao <fujii@postgresql.org>
Thu, 30 Jan 2020 15:42:06 +0000 (00:42 +0900)
committerFujii Masao <fujii@postgresql.org>
Thu, 30 Jan 2020 15:42:06 +0000 (00:42 +0900)
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

src/backend/commands/tablecmds.c
src/test/regress/expected/privileges.out
src/test/regress/sql/privileges.sql

index 70589dd1dc3390170e069e77bc87f1e4936e18d1..c3cda68068397f3cd0e153d5863d2d893a0633c3 100644 (file)
@@ -304,6 +304,7 @@ struct DropRelationCallbackState
    ((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
 
 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 void RangeVarCallbackForTruncate(const RangeVar *relation,
                                        Oid relId, Oid oldRelId, void *arg);
@@ -1615,6 +1616,12 @@ ExecuteTruncate(TruncateStmt *stmt)
                    continue;
                }
 
+               /*
+                * 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);
 
@@ -1701,6 +1708,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
                        (errmsg("truncate cascades to table \"%s\"",
                                RelationGetRelationName(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);
@@ -1951,7 +1959,6 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
 static void
 truncate_check_rel(Oid relid, Form_pg_class reltuple)
 {
-   AclResult   aclresult;
    char       *relname = NameStr(reltuple->relname);
 
    /*
@@ -1965,12 +1972,6 @@ truncate_check_rel(Oid relid, Form_pg_class reltuple)
                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                 errmsg("\"%s\" is not a table", relname)));
 
-   /* Permissions checks */
-   aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
-   if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, get_relkind_objtype(reltuple->relkind),
-                      relname);
-
    if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -1980,6 +1981,22 @@ truncate_check_rel(Oid relid, Form_pg_class reltuple)
    InvokeObjectTruncateHook(relid);
 }
 
+/*
+ * 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.  This is split with truncate_check_rel() as
@@ -15292,6 +15309,7 @@ RangeVarCallbackForTruncate(const RangeVar *relation,
        elog(ERROR, "cache lookup failed for relation %u", relId);
 
    truncate_check_rel(relId, (Form_pg_class) GETSTRUCT(tuple));
+   truncate_check_perms(relId, (Form_pg_class) GETSTRUCT(tuple));
 
    ReleaseSysCache(tuple);
 }
index 6674f268d7c5177295fa675566b5396b24632e4c..297cedbacf11df1bb8c72f4746b84dff0f8a9c64 100644 (file)
@@ -695,6 +695,27 @@ SELECT tableoid 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 -
index f15d1f377370bdcb637fb9bb07cd076ff74ef6f2..dfe2603fe2ebfc70685d6487a0d5f9a4919c2efd 100644 (file)
@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
 SELECT atestp2 FROM atestp2; -- ok
 SELECT tableoid 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