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

Commit01c7a87

Browse files
committed
Fix corner-case failures in has_foo_privilege() family of functions.
The variants of these functions that take numeric inputs (OIDs orcolumn numbers) are supposed to return NULL rather than failingon bad input; this rule reduces problems with snapshot skew whenqueries apply the functions to all rows of a catalog.has_column_privilege() had careless handling of the case where thetable OID didn't exist. You might get something like this:select has_column_privilege(9999,'nosuchcol','select');ERROR: column "nosuchcol" of relation "(null)" does not existor you might get a crash, depending on the platform's printf's responseto a null string pointer.In addition, while applying the column-number variant to a droppedcolumn returned NULL as desired, applying the column-name variantdid not:select has_column_privilege('mytable','........pg.dropped.2........','select');ERROR: column "........pg.dropped.2........" of relation "mytable" does not existIt seems better to make this case return NULL as well.Also, the OID-accepting variants of has_foreign_data_wrapper_privilege,has_server_privilege, and has_tablespace_privilege didn't follow theprinciple of returning NULL for nonexistent OIDs. Superusers got TRUE,everybody else got an error.Per investigation of Jaime Casanova's report of a new crash in HEAD.These behaviors have been like this for a long time, so back-patch toall supported branches.Patch by me; thanks to Stephen Frost for discussion and reviewDiscussion:https://postgr.es/m/CAJGNTeP=-6Gyqq5TN9OvYEydi7Fv1oGyYj650LGTnW44oAzYCg@mail.gmail.com
1 parent4b2fb12 commit01c7a87

File tree

3 files changed

+159
-10
lines changed

3 files changed

+159
-10
lines changed

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

Lines changed: 85 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2450,8 +2450,12 @@ has_any_column_privilege_id_id(PG_FUNCTION_ARGS)
24502450
*
24512451
*The result is a boolean value: true if user has the indicated
24522452
*privilege, false if not. The variants that take a relation OID
2453-
*and an integer attnum return NULL (rather than throwing an error)
2454-
*if the column doesn't exist or is dropped.
2453+
*return NULL (rather than throwing an error) if that relation OID
2454+
*doesn't exist. Likewise, the variants that take an integer attnum
2455+
*return NULL (rather than throwing an error) if there is no such
2456+
*pg_attribute entry. All variants return NULL if an attisdropped
2457+
*column is selected. These rules are meant to avoid unnecessary
2458+
*failures in queries that scan pg_attribute.
24552459
*/
24562460

24572461
/*
@@ -2468,6 +2472,12 @@ column_privilege_check(Oid tableoid, AttrNumber attnum,
24682472
HeapTupleattTuple;
24692473
Form_pg_attributeattributeForm;
24702474

2475+
/*
2476+
* If convert_column_name failed, we can just return -1 immediately.
2477+
*/
2478+
if (attnum==InvalidAttrNumber)
2479+
return-1;
2480+
24712481
/*
24722482
* First check if we have the privilege at the table level. We check
24732483
* existence of the pg_class row before risking calling pg_class_aclcheck.
@@ -2829,21 +2839,59 @@ has_column_privilege_id_attnum(PG_FUNCTION_ARGS)
28292839

28302840
/*
28312841
* Given a table OID and a column name expressed as a string, look it up
2832-
* and return the column number
2842+
* and return the column number. Returns InvalidAttrNumber in cases
2843+
* where caller should return NULL instead of failing.
28332844
*/
28342845
staticAttrNumber
28352846
convert_column_name(Oidtableoid,text*column)
28362847
{
2837-
AttrNumberattnum;
28382848
char*colname;
2849+
HeapTupleattTuple;
2850+
AttrNumberattnum;
28392851

28402852
colname=text_to_cstring(column);
2841-
attnum=get_attnum(tableoid,colname);
2842-
if (attnum==InvalidAttrNumber)
2843-
ereport(ERROR,
2844-
(errcode(ERRCODE_UNDEFINED_COLUMN),
2845-
errmsg("column \"%s\" of relation \"%s\" does not exist",
2846-
colname,get_rel_name(tableoid))));
2853+
2854+
/*
2855+
* We don't use get_attnum() here because it will report that dropped
2856+
* columns don't exist. We need to treat dropped columns differently from
2857+
* nonexistent columns.
2858+
*/
2859+
attTuple=SearchSysCache2(ATTNAME,
2860+
ObjectIdGetDatum(tableoid),
2861+
CStringGetDatum(colname));
2862+
if (HeapTupleIsValid(attTuple))
2863+
{
2864+
Form_pg_attributeattributeForm;
2865+
2866+
attributeForm= (Form_pg_attribute)GETSTRUCT(attTuple);
2867+
/* We want to return NULL for dropped columns */
2868+
if (attributeForm->attisdropped)
2869+
attnum=InvalidAttrNumber;
2870+
else
2871+
attnum=attributeForm->attnum;
2872+
ReleaseSysCache(attTuple);
2873+
}
2874+
else
2875+
{
2876+
char*tablename=get_rel_name(tableoid);
2877+
2878+
/*
2879+
* If the table OID is bogus, or it's just been dropped, we'll get
2880+
* NULL back. In such cases we want has_column_privilege to return
2881+
* NULL too, so just return InvalidAttrNumber.
2882+
*/
2883+
if (tablename!=NULL)
2884+
{
2885+
/* tableoid exists, colname does not, so throw error */
2886+
ereport(ERROR,
2887+
(errcode(ERRCODE_UNDEFINED_COLUMN),
2888+
errmsg("column \"%s\" of relation \"%s\" does not exist",
2889+
colname,tablename)));
2890+
}
2891+
/* tableoid doesn't exist, so act like attisdropped case */
2892+
attnum=InvalidAttrNumber;
2893+
}
2894+
28472895
pfree(colname);
28482896
returnattnum;
28492897
}
@@ -3147,6 +3195,9 @@ has_foreign_data_wrapper_privilege_name_id(PG_FUNCTION_ARGS)
31473195
roleid=get_role_oid_or_public(NameStr(*username));
31483196
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
31493197

3198+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3199+
PG_RETURN_NULL();
3200+
31503201
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
31513202

31523203
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3170,6 +3221,9 @@ has_foreign_data_wrapper_privilege_id(PG_FUNCTION_ARGS)
31703221
roleid=GetUserId();
31713222
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
31723223

3224+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3225+
PG_RETURN_NULL();
3226+
31733227
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
31743228

31753229
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3214,6 +3268,9 @@ has_foreign_data_wrapper_privilege_id_id(PG_FUNCTION_ARGS)
32143268

32153269
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
32163270

3271+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3272+
PG_RETURN_NULL();
3273+
32173274
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
32183275

32193276
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3913,6 +3970,9 @@ has_server_privilege_name_id(PG_FUNCTION_ARGS)
39133970
roleid=get_role_oid_or_public(NameStr(*username));
39143971
mode=convert_server_priv_string(priv_type_text);
39153972

3973+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
3974+
PG_RETURN_NULL();
3975+
39163976
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39173977

39183978
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3936,6 +3996,9 @@ has_server_privilege_id(PG_FUNCTION_ARGS)
39363996
roleid=GetUserId();
39373997
mode=convert_server_priv_string(priv_type_text);
39383998

3999+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
4000+
PG_RETURN_NULL();
4001+
39394002
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39404003

39414004
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3980,6 +4043,9 @@ has_server_privilege_id_id(PG_FUNCTION_ARGS)
39804043

39814044
mode=convert_server_priv_string(priv_type_text);
39824045

4046+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
4047+
PG_RETURN_NULL();
4048+
39834049
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39844050

39854051
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4095,6 +4161,9 @@ has_tablespace_privilege_name_id(PG_FUNCTION_ARGS)
40954161
roleid=get_role_oid_or_public(NameStr(*username));
40964162
mode=convert_tablespace_priv_string(priv_type_text);
40974163

4164+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4165+
PG_RETURN_NULL();
4166+
40984167
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
40994168

41004169
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4118,6 +4187,9 @@ has_tablespace_privilege_id(PG_FUNCTION_ARGS)
41184187
roleid=GetUserId();
41194188
mode=convert_tablespace_priv_string(priv_type_text);
41204189

4190+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4191+
PG_RETURN_NULL();
4192+
41214193
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
41224194

41234195
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4162,6 +4234,9 @@ has_tablespace_privilege_id_id(PG_FUNCTION_ARGS)
41624234

41634235
mode=convert_tablespace_priv_string(priv_type_text);
41644236

4237+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4238+
PG_RETURN_NULL();
4239+
41654240
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
41664241

41674242
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,63 @@ from (select oid from pg_class where relname = 'atest1') as t1;
10361036
f
10371037
(1 row)
10381038

1039+
-- has_column_privilege function
1040+
-- bad-input checks (as non-super-user)
1041+
select has_column_privilege('pg_authid',NULL,'select');
1042+
has_column_privilege
1043+
----------------------
1044+
1045+
(1 row)
1046+
1047+
select has_column_privilege('pg_authid','nosuchcol','select');
1048+
ERROR: column "nosuchcol" of relation "pg_authid" does not exist
1049+
select has_column_privilege(9999,'nosuchcol','select');
1050+
has_column_privilege
1051+
----------------------
1052+
1053+
(1 row)
1054+
1055+
select has_column_privilege(9999,99::int2,'select');
1056+
has_column_privilege
1057+
----------------------
1058+
1059+
(1 row)
1060+
1061+
select has_column_privilege('pg_authid',99::int2,'select');
1062+
has_column_privilege
1063+
----------------------
1064+
1065+
(1 row)
1066+
1067+
select has_column_privilege(9999,99::int2,'select');
1068+
has_column_privilege
1069+
----------------------
1070+
1071+
(1 row)
1072+
1073+
create temp table mytable(f1 int, f2 int, f3 int);
1074+
alter table mytable drop column f2;
1075+
select has_column_privilege('mytable','f2','select');
1076+
ERROR: column "f2" of relation "mytable" does not exist
1077+
select has_column_privilege('mytable','........pg.dropped.2........','select');
1078+
has_column_privilege
1079+
----------------------
1080+
1081+
(1 row)
1082+
1083+
select has_column_privilege('mytable',2::int2,'select');
1084+
has_column_privilege
1085+
----------------------
1086+
t
1087+
(1 row)
1088+
1089+
revoke select on table mytable from regressuser3;
1090+
select has_column_privilege('mytable',2::int2,'select');
1091+
has_column_privilege
1092+
----------------------
1093+
1094+
(1 row)
1095+
10391096
-- Grant options
10401097
SET SESSION AUTHORIZATION regressuser1;
10411098
CREATE TABLE atest4 (a int);

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,23 @@ from (select oid from pg_class where relname = 'atest1') as t1;
657657
select has_table_privilege(t1.oid,'trigger')
658658
from (selectoidfrom pg_classwhere relname='atest1')as t1;
659659

660+
-- has_column_privilege function
661+
662+
-- bad-input checks (as non-super-user)
663+
select has_column_privilege('pg_authid',NULL,'select');
664+
select has_column_privilege('pg_authid','nosuchcol','select');
665+
select has_column_privilege(9999,'nosuchcol','select');
666+
select has_column_privilege(9999,99::int2,'select');
667+
select has_column_privilege('pg_authid',99::int2,'select');
668+
select has_column_privilege(9999,99::int2,'select');
669+
670+
create temp table mytable(f1int, f2int, f3int);
671+
altertable mytable drop column f2;
672+
select has_column_privilege('mytable','f2','select');
673+
select has_column_privilege('mytable','........pg.dropped.2........','select');
674+
select has_column_privilege('mytable',2::int2,'select');
675+
revokeselecton table mytablefrom regressuser3;
676+
select has_column_privilege('mytable',2::int2,'select');
660677

661678
-- Grant options
662679

0 commit comments

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