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

Commit419cc8a

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 parente9cff30 commit419cc8a

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
@@ -2448,8 +2448,12 @@ has_any_column_privilege_id_id(PG_FUNCTION_ARGS)
24482448
*
24492449
*The result is a boolean value: true if user has the indicated
24502450
*privilege, false if not. The variants that take a relation OID
2451-
*and an integer attnum return NULL (rather than throwing an error)
2452-
*if the column doesn't exist or is dropped.
2451+
*return NULL (rather than throwing an error) if that relation OID
2452+
*doesn't exist. Likewise, the variants that take an integer attnum
2453+
*return NULL (rather than throwing an error) if there is no such
2454+
*pg_attribute entry. All variants return NULL if an attisdropped
2455+
*column is selected. These rules are meant to avoid unnecessary
2456+
*failures in queries that scan pg_attribute.
24532457
*/
24542458

24552459
/*
@@ -2466,6 +2470,12 @@ column_privilege_check(Oid tableoid, AttrNumber attnum,
24662470
HeapTupleattTuple;
24672471
Form_pg_attributeattributeForm;
24682472

2473+
/*
2474+
* If convert_column_name failed, we can just return -1 immediately.
2475+
*/
2476+
if (attnum==InvalidAttrNumber)
2477+
return-1;
2478+
24692479
/*
24702480
* First check if we have the privilege at the table level. We check
24712481
* existence of the pg_class row before risking calling pg_class_aclcheck.
@@ -2827,21 +2837,59 @@ has_column_privilege_id_attnum(PG_FUNCTION_ARGS)
28272837

28282838
/*
28292839
* Given a table OID and a column name expressed as a string, look it up
2830-
* and return the column number
2840+
* and return the column number. Returns InvalidAttrNumber in cases
2841+
* where caller should return NULL instead of failing.
28312842
*/
28322843
staticAttrNumber
28332844
convert_column_name(Oidtableoid,text*column)
28342845
{
2835-
AttrNumberattnum;
28362846
char*colname;
2847+
HeapTupleattTuple;
2848+
AttrNumberattnum;
28372849

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

3196+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3197+
PG_RETURN_NULL();
3198+
31483199
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
31493200

31503201
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3168,6 +3219,9 @@ has_foreign_data_wrapper_privilege_id(PG_FUNCTION_ARGS)
31683219
roleid=GetUserId();
31693220
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
31703221

3222+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3223+
PG_RETURN_NULL();
3224+
31713225
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
31723226

31733227
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3212,6 +3266,9 @@ has_foreign_data_wrapper_privilege_id_id(PG_FUNCTION_ARGS)
32123266

32133267
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
32143268

3269+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3270+
PG_RETURN_NULL();
3271+
32153272
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
32163273

32173274
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3911,6 +3968,9 @@ has_server_privilege_name_id(PG_FUNCTION_ARGS)
39113968
roleid=get_role_oid_or_public(NameStr(*username));
39123969
mode=convert_server_priv_string(priv_type_text);
39133970

3971+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
3972+
PG_RETURN_NULL();
3973+
39143974
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39153975

39163976
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3934,6 +3994,9 @@ has_server_privilege_id(PG_FUNCTION_ARGS)
39343994
roleid=GetUserId();
39353995
mode=convert_server_priv_string(priv_type_text);
39363996

3997+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
3998+
PG_RETURN_NULL();
3999+
39374000
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39384001

39394002
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3978,6 +4041,9 @@ has_server_privilege_id_id(PG_FUNCTION_ARGS)
39784041

39794042
mode=convert_server_priv_string(priv_type_text);
39804043

4044+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
4045+
PG_RETURN_NULL();
4046+
39814047
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39824048

39834049
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4093,6 +4159,9 @@ has_tablespace_privilege_name_id(PG_FUNCTION_ARGS)
40934159
roleid=get_role_oid_or_public(NameStr(*username));
40944160
mode=convert_tablespace_priv_string(priv_type_text);
40954161

4162+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4163+
PG_RETURN_NULL();
4164+
40964165
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
40974166

40984167
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4116,6 +4185,9 @@ has_tablespace_privilege_id(PG_FUNCTION_ARGS)
41164185
roleid=GetUserId();
41174186
mode=convert_tablespace_priv_string(priv_type_text);
41184187

4188+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4189+
PG_RETURN_NULL();
4190+
41194191
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
41204192

41214193
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4160,6 +4232,9 @@ has_tablespace_privilege_id_id(PG_FUNCTION_ARGS)
41604232

41614233
mode=convert_tablespace_priv_string(priv_type_text);
41624234

4235+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4236+
PG_RETURN_NULL();
4237+
41634238
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
41644239

41654240
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
@@ -1132,6 +1132,63 @@ from (select oid from pg_class where relname = 'atest1') as t1;
11321132
f
11331133
(1 row)
11341134

1135+
-- has_column_privilege function
1136+
-- bad-input checks (as non-super-user)
1137+
select has_column_privilege('pg_authid',NULL,'select');
1138+
has_column_privilege
1139+
----------------------
1140+
1141+
(1 row)
1142+
1143+
select has_column_privilege('pg_authid','nosuchcol','select');
1144+
ERROR: column "nosuchcol" of relation "pg_authid" does not exist
1145+
select has_column_privilege(9999,'nosuchcol','select');
1146+
has_column_privilege
1147+
----------------------
1148+
1149+
(1 row)
1150+
1151+
select has_column_privilege(9999,99::int2,'select');
1152+
has_column_privilege
1153+
----------------------
1154+
1155+
(1 row)
1156+
1157+
select has_column_privilege('pg_authid',99::int2,'select');
1158+
has_column_privilege
1159+
----------------------
1160+
1161+
(1 row)
1162+
1163+
select has_column_privilege(9999,99::int2,'select');
1164+
has_column_privilege
1165+
----------------------
1166+
1167+
(1 row)
1168+
1169+
create temp table mytable(f1 int, f2 int, f3 int);
1170+
alter table mytable drop column f2;
1171+
select has_column_privilege('mytable','f2','select');
1172+
ERROR: column "f2" of relation "mytable" does not exist
1173+
select has_column_privilege('mytable','........pg.dropped.2........','select');
1174+
has_column_privilege
1175+
----------------------
1176+
1177+
(1 row)
1178+
1179+
select has_column_privilege('mytable',2::int2,'select');
1180+
has_column_privilege
1181+
----------------------
1182+
t
1183+
(1 row)
1184+
1185+
revoke select on table mytable from regress_priv_user3;
1186+
select has_column_privilege('mytable',2::int2,'select');
1187+
has_column_privilege
1188+
----------------------
1189+
1190+
(1 row)
1191+
11351192
-- Grant options
11361193
SET SESSION AUTHORIZATION regress_priv_user1;
11371194
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
@@ -715,6 +715,23 @@ from (select oid from pg_class where relname = 'atest1') as t1;
715715
select has_table_privilege(t1.oid,'trigger')
716716
from (selectoidfrom pg_classwhere relname='atest1')as t1;
717717

718+
-- has_column_privilege function
719+
720+
-- bad-input checks (as non-super-user)
721+
select has_column_privilege('pg_authid',NULL,'select');
722+
select has_column_privilege('pg_authid','nosuchcol','select');
723+
select has_column_privilege(9999,'nosuchcol','select');
724+
select has_column_privilege(9999,99::int2,'select');
725+
select has_column_privilege('pg_authid',99::int2,'select');
726+
select has_column_privilege(9999,99::int2,'select');
727+
728+
create temp table mytable(f1int, f2int, f3int);
729+
altertable mytable drop column f2;
730+
select has_column_privilege('mytable','f2','select');
731+
select has_column_privilege('mytable','........pg.dropped.2........','select');
732+
select has_column_privilege('mytable',2::int2,'select');
733+
revokeselecton table mytablefrom regress_priv_user3;
734+
select has_column_privilege('mytable',2::int2,'select');
718735

719736
-- Grant options
720737

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp