Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitb12bd48

Browse files
committed
Fix has_column_privilege function corner case
According to the comments, when an invalid or dropped column oid is passedto has_column_privilege(), the intention has always been to return NULL.However, when the caller had table level privilege the invalid/missingcolumn was never discovered, because table permissions were checked first.Fix that by introducing extended versions of pg_attribute_acl(check|mask)and pg_class_acl(check|mask) which take a new argument, is_missing. Whenis_missing is NULL, the old behavior is preserved. But when is_missing ispassed by the caller, no ERROR is thrown for dropped or missingcolumns/relations, and is_missing is flipped to true. This in turn allowshas_column_privilege to check for column privileges first, providing thedesired semantics.Not backpatched since it is a user visible behavioral change with no previouscomplaints, and the fix is a bit on the invasive side.Author: Joe ConwayReviewed-By: Tom LaneReported by: Ian BarwickDiscussion:https://postgr.es/m/flat/9b5f4311-157b-4164-7fe7-077b4fe8ed84%40joeconway.com
1 parent86dc900 commitb12bd48

File tree

5 files changed

+142
-49
lines changed

5 files changed

+142
-49
lines changed

‎src/backend/catalog/aclchk.c‎

Lines changed: 101 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3705,6 +3705,20 @@ pg_aclmask(ObjectType objtype, Oid table_oid, AttrNumber attnum, Oid roleid,
37053705
AclMode
37063706
pg_attribute_aclmask(Oidtable_oid,AttrNumberattnum,Oidroleid,
37073707
AclModemask,AclMaskHowhow)
3708+
{
3709+
returnpg_attribute_aclmask_ext(table_oid,attnum,roleid,
3710+
mask,how,NULL);
3711+
}
3712+
3713+
/*
3714+
* Exported routine for examining a user's privileges for a column
3715+
*
3716+
* Does the bulk of the work for pg_attribute_aclmask(), and allows other
3717+
* callers to avoid the missing attribute ERROR when is_missing is non-NULL.
3718+
*/
3719+
AclMode
3720+
pg_attribute_aclmask_ext(Oidtable_oid,AttrNumberattnum,Oidroleid,
3721+
AclModemask,AclMaskHowhow,bool*is_missing)
37083722
{
37093723
AclModeresult;
37103724
HeapTupleclassTuple;
@@ -3723,18 +3737,38 @@ pg_attribute_aclmask(Oid table_oid, AttrNumber attnum, Oid roleid,
37233737
ObjectIdGetDatum(table_oid),
37243738
Int16GetDatum(attnum));
37253739
if (!HeapTupleIsValid(attTuple))
3726-
ereport(ERROR,
3727-
(errcode(ERRCODE_UNDEFINED_COLUMN),
3728-
errmsg("attribute %d of relation with OID %u does not exist",
3729-
attnum,table_oid)));
3740+
{
3741+
if (is_missing!=NULL)
3742+
{
3743+
/* return "no privileges" instead of throwing an error */
3744+
*is_missing= true;
3745+
return0;
3746+
}
3747+
else
3748+
ereport(ERROR,
3749+
(errcode(ERRCODE_UNDEFINED_COLUMN),
3750+
errmsg("attribute %d of relation with OID %u does not exist",
3751+
attnum,table_oid)));
3752+
}
3753+
37303754
attributeForm= (Form_pg_attribute)GETSTRUCT(attTuple);
37313755

3732-
/*Throw error on dropped columns, too */
3756+
/*Check dropped columns, too */
37333757
if (attributeForm->attisdropped)
3734-
ereport(ERROR,
3735-
(errcode(ERRCODE_UNDEFINED_COLUMN),
3736-
errmsg("attribute %d of relation with OID %u does not exist",
3737-
attnum,table_oid)));
3758+
{
3759+
if (is_missing!=NULL)
3760+
{
3761+
/* return "no privileges" instead of throwing an error */
3762+
*is_missing= true;
3763+
ReleaseSysCache(attTuple);
3764+
return0;
3765+
}
3766+
else
3767+
ereport(ERROR,
3768+
(errcode(ERRCODE_UNDEFINED_COLUMN),
3769+
errmsg("attribute %d of relation with OID %u does not exist",
3770+
attnum,table_oid)));
3771+
}
37383772

37393773
aclDatum=SysCacheGetAttr(ATTNUM,attTuple,Anum_pg_attribute_attacl,
37403774
&isNull);
@@ -3790,6 +3824,19 @@ pg_attribute_aclmask(Oid table_oid, AttrNumber attnum, Oid roleid,
37903824
AclMode
37913825
pg_class_aclmask(Oidtable_oid,Oidroleid,
37923826
AclModemask,AclMaskHowhow)
3827+
{
3828+
returnpg_class_aclmask_ext(table_oid,roleid,mask,how,NULL);
3829+
}
3830+
3831+
/*
3832+
* Exported routine for examining a user's privileges for a table
3833+
*
3834+
* Does the bulk of the work for pg_class_aclmask(), and allows other
3835+
* callers to avoid the missing relation ERROR when is_missing is non-NULL.
3836+
*/
3837+
AclMode
3838+
pg_class_aclmask_ext(Oidtable_oid,Oidroleid,AclModemask,
3839+
AclMaskHowhow,bool*is_missing)
37933840
{
37943841
AclModeresult;
37953842
HeapTupletuple;
@@ -3804,10 +3851,20 @@ pg_class_aclmask(Oid table_oid, Oid roleid,
38043851
*/
38053852
tuple=SearchSysCache1(RELOID,ObjectIdGetDatum(table_oid));
38063853
if (!HeapTupleIsValid(tuple))
3807-
ereport(ERROR,
3808-
(errcode(ERRCODE_UNDEFINED_TABLE),
3809-
errmsg("relation with OID %u does not exist",
3810-
table_oid)));
3854+
{
3855+
if (is_missing!=NULL)
3856+
{
3857+
/* return "no privileges" instead of throwing an error */
3858+
*is_missing= true;
3859+
return0;
3860+
}
3861+
else
3862+
ereport(ERROR,
3863+
(errcode(ERRCODE_UNDEFINED_TABLE),
3864+
errmsg("relation with OID %u does not exist",
3865+
table_oid)));
3866+
}
3867+
38113868
classForm= (Form_pg_class)GETSTRUCT(tuple);
38123869

38133870
/*
@@ -4468,7 +4525,22 @@ AclResult
44684525
pg_attribute_aclcheck(Oidtable_oid,AttrNumberattnum,
44694526
Oidroleid,AclModemode)
44704527
{
4471-
if (pg_attribute_aclmask(table_oid,attnum,roleid,mode,ACLMASK_ANY)!=0)
4528+
returnpg_attribute_aclcheck_ext(table_oid,attnum,roleid,mode,NULL);
4529+
}
4530+
4531+
4532+
/*
4533+
* Exported routine for checking a user's access privileges to a column
4534+
*
4535+
* Does the bulk of the work for pg_attribute_aclcheck(), and allows other
4536+
* callers to avoid the missing attribute ERROR when is_missing is non-NULL.
4537+
*/
4538+
AclResult
4539+
pg_attribute_aclcheck_ext(Oidtable_oid,AttrNumberattnum,
4540+
Oidroleid,AclModemode,bool*is_missing)
4541+
{
4542+
if (pg_attribute_aclmask_ext(table_oid,attnum,roleid,mode,
4543+
ACLMASK_ANY,is_missing)!=0)
44724544
returnACLCHECK_OK;
44734545
else
44744546
returnACLCHECK_NO_PRIV;
@@ -4581,7 +4653,21 @@ pg_attribute_aclcheck_all(Oid table_oid, Oid roleid, AclMode mode,
45814653
AclResult
45824654
pg_class_aclcheck(Oidtable_oid,Oidroleid,AclModemode)
45834655
{
4584-
if (pg_class_aclmask(table_oid,roleid,mode,ACLMASK_ANY)!=0)
4656+
returnpg_class_aclcheck_ext(table_oid,roleid,mode,NULL);
4657+
}
4658+
4659+
/*
4660+
* Exported routine for checking a user's access privileges to a table
4661+
*
4662+
* Does the bulk of the work for pg_class_aclcheck(), and allows other
4663+
* callers to avoid the missing relation ERROR when is_missing is non-NULL.
4664+
*/
4665+
AclResult
4666+
pg_class_aclcheck_ext(Oidtable_oid,Oidroleid,
4667+
AclModemode,bool*is_missing)
4668+
{
4669+
if (pg_class_aclmask_ext(table_oid,roleid,mode,
4670+
ACLMASK_ANY,is_missing)!=0)
45854671
returnACLCHECK_OK;
45864672
else
45874673
returnACLCHECK_NO_PRIV;

‎src/backend/utils/adt/acl.c‎

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2444,8 +2444,7 @@ column_privilege_check(Oid tableoid, AttrNumber attnum,
24442444
Oidroleid,AclModemode)
24452445
{
24462446
AclResultaclresult;
2447-
HeapTupleattTuple;
2448-
Form_pg_attributeattributeForm;
2447+
boolis_missing= false;
24492448

24502449
/*
24512450
* If convert_column_name failed, we can just return -1 immediately.
@@ -2454,42 +2453,25 @@ column_privilege_check(Oid tableoid, AttrNumber attnum,
24542453
return-1;
24552454

24562455
/*
2457-
* First check if we have the privilege at the table level. We check
2458-
* existence of the pg_class row before risking calling pg_class_aclcheck.
2459-
* Note: it might seem there's a race condition against concurrent DROP,
2460-
* but really it's safe because there will be no syscache flush between
2461-
* here and there. So if we see the row in the syscache, so will
2462-
* pg_class_aclcheck.
2456+
* Check for column-level privileges first. This serves in
2457+
* part as a check on whether the column even exists, so we
2458+
* need to do it before checking table-level privilege.
24632459
*/
2464-
if (!SearchSysCacheExists1(RELOID,ObjectIdGetDatum(tableoid)))
2460+
aclresult=pg_attribute_aclcheck_ext(tableoid,attnum,roleid,
2461+
mode,&is_missing);
2462+
if (aclresult==ACLCHECK_OK)
2463+
return1;
2464+
elseif (is_missing)
24652465
return-1;
24662466

2467-
aclresult=pg_class_aclcheck(tableoid,roleid,mode);
2468-
2467+
/* Next check if we have the privilege at the table level */
2468+
aclresult=pg_class_aclcheck_ext(tableoid,roleid,mode,&is_missing);
24692469
if (aclresult==ACLCHECK_OK)
2470-
return true;
2471-
2472-
/*
2473-
* No table privilege, so try per-column privileges. Again, we have to
2474-
* check for dropped attribute first, and we rely on the syscache not to
2475-
* notice a concurrent drop before pg_attribute_aclcheck fetches the row.
2476-
*/
2477-
attTuple=SearchSysCache2(ATTNUM,
2478-
ObjectIdGetDatum(tableoid),
2479-
Int16GetDatum(attnum));
2480-
if (!HeapTupleIsValid(attTuple))
2481-
return-1;
2482-
attributeForm= (Form_pg_attribute)GETSTRUCT(attTuple);
2483-
if (attributeForm->attisdropped)
2484-
{
2485-
ReleaseSysCache(attTuple);
2470+
return1;
2471+
elseif (is_missing)
24862472
return-1;
2487-
}
2488-
ReleaseSysCache(attTuple);
2489-
2490-
aclresult=pg_attribute_aclcheck(tableoid,attnum,roleid,mode);
2491-
2492-
return (aclresult==ACLCHECK_OK);
2473+
else
2474+
return0;
24932475
}
24942476

24952477
/*

‎src/include/utils/acl.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,14 @@ extern void RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid);
233233

234234
externAclModepg_attribute_aclmask(Oidtable_oid,AttrNumberattnum,
235235
Oidroleid,AclModemask,AclMaskHowhow);
236+
externAclModepg_attribute_aclmask_ext(Oidtable_oid,AttrNumberattnum,
237+
Oidroleid,AclModemask,
238+
AclMaskHowhow,bool*is_missing);
236239
externAclModepg_class_aclmask(Oidtable_oid,Oidroleid,
237240
AclModemask,AclMaskHowhow);
241+
externAclModepg_class_aclmask_ext(Oidtable_oid,Oidroleid,
242+
AclModemask,AclMaskHowhow,
243+
bool*is_missing);
238244
externAclModepg_database_aclmask(Oiddb_oid,Oidroleid,
239245
AclModemask,AclMaskHowhow);
240246
externAclModepg_proc_aclmask(Oidproc_oid,Oidroleid,
@@ -256,9 +262,14 @@ extern AclMode pg_type_aclmask(Oid type_oid, Oid roleid,
256262

257263
externAclResultpg_attribute_aclcheck(Oidtable_oid,AttrNumberattnum,
258264
Oidroleid,AclModemode);
265+
externAclResultpg_attribute_aclcheck_ext(Oidtable_oid,AttrNumberattnum,
266+
Oidroleid,AclModemode,
267+
bool*is_missing);
259268
externAclResultpg_attribute_aclcheck_all(Oidtable_oid,Oidroleid,
260269
AclModemode,AclMaskHowhow);
261270
externAclResultpg_class_aclcheck(Oidtable_oid,Oidroleid,AclModemode);
271+
externAclResultpg_class_aclcheck_ext(Oidtable_oid,Oidroleid,
272+
AclModemode,bool*is_missing);
262273
externAclResultpg_database_aclcheck(Oiddb_oid,Oidroleid,AclModemode);
263274
externAclResultpg_proc_aclcheck(Oidproc_oid,Oidroleid,AclModemode);
264275
externAclResultpg_language_aclcheck(Oidlang_oid,Oidroleid,AclModemode);

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1362,7 +1362,13 @@ select has_column_privilege('mytable','........pg.dropped.2........','select');
13621362
select has_column_privilege('mytable',2::int2,'select');
13631363
has_column_privilege
13641364
----------------------
1365-
t
1365+
1366+
(1 row)
1367+
1368+
select has_column_privilege('mytable',99::int2,'select');
1369+
has_column_privilege
1370+
----------------------
1371+
13661372
(1 row)
13671373

13681374
revoke select on table mytable from regress_priv_user3;
@@ -1372,6 +1378,12 @@ select has_column_privilege('mytable',2::int2,'select');
13721378

13731379
(1 row)
13741380

1381+
select has_column_privilege('mytable',99::int2,'select');
1382+
has_column_privilege
1383+
----------------------
1384+
1385+
(1 row)
1386+
13751387
drop table mytable;
13761388
-- Grant options
13771389
SET SESSION AUTHORIZATION regress_priv_user1;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,8 +836,10 @@ alter table mytable drop column f2;
836836
select has_column_privilege('mytable','f2','select');
837837
select has_column_privilege('mytable','........pg.dropped.2........','select');
838838
select has_column_privilege('mytable',2::int2,'select');
839+
select has_column_privilege('mytable',99::int2,'select');
839840
revokeselecton table mytablefrom regress_priv_user3;
840841
select has_column_privilege('mytable',2::int2,'select');
842+
select has_column_privilege('mytable',99::int2,'select');
841843
droptable mytable;
842844

843845
-- Grant options

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp