From 8b1a6499d055f6f222e200fd6410073537d11f31 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Mon, 3 Feb 2020 12:41:40 +0900 Subject: [PATCH] Revert commit 4b96c03a0a. This commit reverts the fix "Make inherited TRUNCATE perform access permission checks on parent table only" only in the back branches. It's not hard to imagine that there are some applications expecting the old behavior and the fix breaks their security. To avoid this compatibility problem, we decided to apply the fix only in HEAD and revert it in all supported back branches. Discussion: https://postgr.es/m/21015.1580400165@sss.pgh.pa.us --- src/backend/commands/tablecmds.c | 69 +++++++----------------- src/test/regress/expected/privileges.out | 21 -------- src/test/regress/sql/privileges.sql | 14 ----- 3 files changed, 19 insertions(+), 85 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 204d25aeb39..1a005760d86 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -291,9 +291,7 @@ struct DropRelationCallbackState #define child_dependency_type(child_is_partition) \ ((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 truncate_check_rel(Relation rel); static List *MergeAttributes(List *schema, List *supers, char relpersistence, bool is_partition, List **supOids, List **supconstr, int *supOidCount); @@ -1244,11 +1242,7 @@ ExecuteTruncate(TruncateStmt *stmt) heap_close(rel, lockmode); continue; } - - truncate_check_rel(myrelid, rel->rd_rel); - truncate_check_perms(myrelid, rel->rd_rel); - truncate_check_activity(rel); - + truncate_check_rel(rel); rels = lappend(rels, rel); relids = lappend_oid(relids, myrelid); @@ -1284,15 +1278,7 @@ 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); - + truncate_check_rel(rel); rels = lappend(rels, rel); relids = lappend_oid(relids, childrelid); } @@ -1331,9 +1317,7 @@ ExecuteTruncate(TruncateStmt *stmt) ereport(NOTICE, (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); + truncate_check_rel(rel); rels = lappend(rels, rel); relids = lappend_oid(relids, relid); } @@ -1546,50 +1530,35 @@ ExecuteTruncate(TruncateStmt *stmt) * Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate */ static void -truncate_check_rel(Oid relid, Form_pg_class reltuple) +truncate_check_rel(Relation rel) { - char *relname = NameStr(reltuple->relname); + AclResult aclresult; /* * 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 (reltuple->relkind != RELKIND_RELATION && - reltuple->relkind != RELKIND_PARTITIONED_TABLE) + if (rel->rd_rel->relkind != RELKIND_RELATION && + rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table", relname))); + errmsg("\"%s\" is not a table", + RelationGetRelationName(rel)))); - if (!allowSystemTableMods && IsSystemClass(relid, reltuple)) + /* Permissions checks */ + aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(), + ACL_TRUNCATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_CLASS, + RelationGetRelationName(rel)); + + if (!allowSystemTableMods && IsSystemRelation(rel)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", - relname))); -} + RelationGetRelationName(rel)))); -/* - * 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, ACL_KIND_CLASS, 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 e242137549d..dacc9845051 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -695,27 +695,6 @@ SELECT oid FROM atestp2; -- ok ----- (0 rows) --- child's permissions do not apply when operating on parent -SET SESSION AUTHORIZATION regress_user1; -REVOKE ALL ON atestc FROM regress_user2; -GRANT ALL ON atestp1 TO regress_user2; -SET SESSION AUTHORIZATION regress_user2; -SELECT f2 FROM atestp1; -- ok - f2 ----- -(0 rows) - -SELECT f2 FROM atestc; -- fail -ERROR: permission denied for relation atestc -DELETE FROM atestp1; -- ok -DELETE FROM atestc; -- fail -ERROR: permission denied for relation atestc -UPDATE atestp1 SET f1 = 1; -- ok -UPDATE atestc SET f1 = 1; -- fail -ERROR: permission denied for relation atestc -TRUNCATE atestp1; -- ok -TRUNCATE atestc; -- fail -ERROR: permission denied for relation 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 973dde81431..4263315a2d8 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -446,20 +446,6 @@ 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_user1; -REVOKE ALL ON atestc FROM regress_user2; -GRANT ALL ON atestp1 TO regress_user2; -SET SESSION AUTHORIZATION regress_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