Skip to content

Commit 8b1a649

Browse files
MasaoFujiiMasaoFujii
committed
Revert commit 4b96c03.
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
1 parent aab30cd commit 8b1a649

File tree

3 files changed

+19
-85
lines changed

3 files changed

+19
-85
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 19 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,7 @@ struct DropRelationCallbackState
291291
#define child_dependency_type(child_is_partition) \
292292
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
293293

294-
static void truncate_check_rel(Oid relid, Form_pg_class reltuple);
295-
static void truncate_check_perms(Oid relid, Form_pg_class reltuple);
296-
static void truncate_check_activity(Relation rel);
294+
static void truncate_check_rel(Relation rel);
297295
static List *MergeAttributes(List *schema, List *supers, char relpersistence,
298296
bool is_partition, List **supOids, List **supconstr,
299297
int *supOidCount);
@@ -1244,11 +1242,7 @@ ExecuteTruncate(TruncateStmt *stmt)
12441242
heap_close(rel, lockmode);
12451243
continue;
12461244
}
1247-
1248-
truncate_check_rel(myrelid, rel->rd_rel);
1249-
truncate_check_perms(myrelid, rel->rd_rel);
1250-
truncate_check_activity(rel);
1251-
1245+
truncate_check_rel(rel);
12521246
rels = lappend(rels, rel);
12531247
relids = lappend_oid(relids, myrelid);
12541248

@@ -1284,15 +1278,7 @@ ExecuteTruncate(TruncateStmt *stmt)
12841278
continue;
12851279
}
12861280

1287-
/*
1288-
* Inherited TRUNCATE commands perform access
1289-
* permission checks on the parent table only.
1290-
* So we skip checking the children's permissions
1291-
* and don't call truncate_check_perms() here.
1292-
*/
1293-
truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);
1294-
truncate_check_activity(rel);
1295-
1281+
truncate_check_rel(rel);
12961282
rels = lappend(rels, rel);
12971283
relids = lappend_oid(relids, childrelid);
12981284
}
@@ -1331,9 +1317,7 @@ ExecuteTruncate(TruncateStmt *stmt)
13311317
ereport(NOTICE,
13321318
(errmsg("truncate cascades to table \"%s\"",
13331319
RelationGetRelationName(rel))));
1334-
truncate_check_rel(relid, rel->rd_rel);
1335-
truncate_check_perms(relid, rel->rd_rel);
1336-
truncate_check_activity(rel);
1320+
truncate_check_rel(rel);
13371321
rels = lappend(rels, rel);
13381322
relids = lappend_oid(relids, relid);
13391323
}
@@ -1546,50 +1530,35 @@ ExecuteTruncate(TruncateStmt *stmt)
15461530
* Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate
15471531
*/
15481532
static void
1549-
truncate_check_rel(Oid relid, Form_pg_class reltuple)
1533+
truncate_check_rel(Relation rel)
15501534
{
1551-
char *relname = NameStr(reltuple->relname);
1535+
AclResult aclresult;
15521536

15531537
/*
15541538
* Only allow truncate on regular tables and partitioned tables (although,
15551539
* the latter are only being included here for the following checks; no
15561540
* physical truncation will occur in their case.)
15571541
*/
1558-
if (reltuple->relkind != RELKIND_RELATION &&
1559-
reltuple->relkind != RELKIND_PARTITIONED_TABLE)
1542+
if (rel->rd_rel->relkind != RELKIND_RELATION &&
1543+
rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
15601544
ereport(ERROR,
15611545
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
1562-
errmsg("\"%s\" is not a table", relname)));
1546+
errmsg("\"%s\" is not a table",
1547+
RelationGetRelationName(rel))));
1548+
1549+
/* Permissions checks */
1550+
aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
1551+
ACL_TRUNCATE);
1552+
if (aclresult != ACLCHECK_OK)
1553+
aclcheck_error(aclresult, ACL_KIND_CLASS,
1554+
RelationGetRelationName(rel));
15631555

1564-
if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
1556+
if (!allowSystemTableMods && IsSystemRelation(rel))
15651557
ereport(ERROR,
15661558
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
15671559
errmsg("permission denied: \"%s\" is a system catalog",
1568-
relname)));
1569-
}
1570-
1571-
/*
1572-
* Check that current user has the permission to truncate given relation.
1573-
*/
1574-
static void
1575-
truncate_check_perms(Oid relid, Form_pg_class reltuple)
1576-
{
1577-
char *relname = NameStr(reltuple->relname);
1578-
AclResult aclresult;
1579-
1580-
/* Permissions checks */
1581-
aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
1582-
if (aclresult != ACLCHECK_OK)
1583-
aclcheck_error(aclresult, ACL_KIND_CLASS, relname);
1584-
}
1560+
RelationGetRelationName(rel))));
15851561

1586-
/*
1587-
* Set of extra sanity checks to check if a given relation is safe to
1588-
* truncate.
1589-
*/
1590-
static void
1591-
truncate_check_activity(Relation rel)
1592-
{
15931562
/*
15941563
* Don't allow truncate on temp tables of other backends ... their local
15951564
* buffer manager is not going to cope.

‎src/test/regress/expected/privileges.out

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -695,27 +695,6 @@ SELECT oid FROM atestp2; -- ok
695695
-----
696696
(0 rows)
697697

698-
-- child's permissions do not apply when operating on parent
699-
SET SESSION AUTHORIZATION regress_user1;
700-
REVOKE ALL ON atestc FROM regress_user2;
701-
GRANT ALL ON atestp1 TO regress_user2;
702-
SET SESSION AUTHORIZATION regress_user2;
703-
SELECT f2 FROM atestp1; -- ok
704-
f2
705-
----
706-
(0 rows)
707-
708-
SELECT f2 FROM atestc; -- fail
709-
ERROR: permission denied for relation atestc
710-
DELETE FROM atestp1; -- ok
711-
DELETE FROM atestc; -- fail
712-
ERROR: permission denied for relation atestc
713-
UPDATE atestp1 SET f1 = 1; -- ok
714-
UPDATE atestc SET f1 = 1; -- fail
715-
ERROR: permission denied for relation atestc
716-
TRUNCATE atestp1; -- ok
717-
TRUNCATE atestc; -- fail
718-
ERROR: permission denied for relation atestc
719698
-- privileges on functions, languages
720699
-- switch to superuser
721700
\c -

‎src/test/regress/sql/privileges.sql

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -446,20 +446,6 @@ SELECT fy FROM atestp2; -- ok
446446
SELECT atestp2 FROM atestp2; -- ok
447447
SELECT oid FROM atestp2; -- ok
448448

449-
-- child's permissions do not apply when operating on parent
450-
SET SESSION AUTHORIZATION regress_user1;
451-
REVOKE ALL ON atestc FROM regress_user2;
452-
GRANT ALL ON atestp1 TO regress_user2;
453-
SET SESSION AUTHORIZATION regress_user2;
454-
SELECT f2 FROM atestp1; -- ok
455-
SELECT f2 FROM atestc; -- fail
456-
DELETE FROM atestp1; -- ok
457-
DELETE FROM atestc; -- fail
458-
UPDATE atestp1 SET f1 = 1; -- ok
459-
UPDATE atestc SET f1 = 1; -- fail
460-
TRUNCATE atestp1; -- ok
461-
TRUNCATE atestc; -- fail
462-
463449
-- privileges on functions, languages
464450

465451
-- switch to superuser

0 commit comments

Comments
 (0)