postgres.git / summary / log / commit / refs

commit    928e755d22de56e998b0b76596ebda1941e0e323
Author:   Fujii Masao <fujii@postgresql.org>
Date:     Thu Jan 30 15:45:39 2020 +0000
First released in: 13.0, REL9_4_STABLE (reverted before release), REL9_5_STABLE (reverted before release), REL9_6_STABLE (reverted before release), REL_10_STABLE (reverted before release), REL_11_STABLE (reverted before release), REL_12_STABLE (reverted before release)
Reverted by: d034ab0bb2ae (in 9.4.26), dc06d0839a33 (in 9.5.21), c15b17f9276e (in 9.6.17), 8b1a6499d055 (in 10.12), ea7857dddb54 (in 11.7), 0d9f307cf82a (in 12.2)

    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


src/backend/commands/tablecmds.c | 67 +++++++++++++++++++++++--------- src/test/regress/expected/privileges.out | 21 ++++++++++ src/test/regress/sql/privileges.sql | 14 +++++++ 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9c6ca7ff9c7..31bc431d9d3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -271,7 +271,9 @@ struct DropRelationCallbackState #define ATT_COMPOSITE_TYPE 0x0010 #define ATT_FOREIGN_TABLE 0x0020 -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, List **supOids, List **supconstr, int *supOidCount); static bool MergeCheckConstraint(List *constraints, char *name, Node *expr); @@ -1050,7 +1052,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); @@ -1086,7 +1092,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); } @@ -1120,7 +1134,9 @@ ExecuteTruncate(TruncateStmt *stmt) 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); } @@ -1328,30 +1344,45 @@ ExecuteTruncate(TruncateStmt *stmt) * 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 */ - if (rel->rd_rel->relkind != RELKIND_RELATION) + if (reltuple->relkind != RELKIND_RELATION) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table", - RelationGetRelationName(rel)))); + errmsg("\"%s\" is not a table", relname))); - /* 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)) + 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, 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 b46af7c5e6a..34f1e74f6ab 100644 --- 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_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 c7d7347091b..c802f190d50 100644 --- 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_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 [parent: 43a648f57ce3]