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

Commit7eed723

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 parent5dd7f5c commit7eed723

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
@@ -2433,8 +2433,12 @@ has_any_column_privilege_id_id(PG_FUNCTION_ARGS)
24332433
*
24342434
*The result is a boolean value: true if user has the indicated
24352435
*privilege, false if not. The variants that take a relation OID
2436-
*and an integer attnum return NULL (rather than throwing an error)
2437-
*if the column doesn't exist or is dropped.
2436+
*return NULL (rather than throwing an error) if that relation OID
2437+
*doesn't exist. Likewise, the variants that take an integer attnum
2438+
*return NULL (rather than throwing an error) if there is no such
2439+
*pg_attribute entry. All variants return NULL if an attisdropped
2440+
*column is selected. These rules are meant to avoid unnecessary
2441+
*failures in queries that scan pg_attribute.
24382442
*/
24392443

24402444
/*
@@ -2451,6 +2455,12 @@ column_privilege_check(Oid tableoid, AttrNumber attnum,
24512455
HeapTupleattTuple;
24522456
Form_pg_attributeattributeForm;
24532457

2458+
/*
2459+
* If convert_column_name failed, we can just return -1 immediately.
2460+
*/
2461+
if (attnum==InvalidAttrNumber)
2462+
return-1;
2463+
24542464
/*
24552465
* First check if we have the privilege at the table level. We check
24562466
* existence of the pg_class row before risking calling pg_class_aclcheck.
@@ -2812,21 +2822,59 @@ has_column_privilege_id_attnum(PG_FUNCTION_ARGS)
28122822

28132823
/*
28142824
* Given a table OID and a column name expressed as a string, look it up
2815-
* and return the column number
2825+
* and return the column number. Returns InvalidAttrNumber in cases
2826+
* where caller should return NULL instead of failing.
28162827
*/
28172828
staticAttrNumber
28182829
convert_column_name(Oidtableoid,text*column)
28192830
{
2820-
AttrNumberattnum;
28212831
char*colname;
2832+
HeapTupleattTuple;
2833+
AttrNumberattnum;
28222834

28232835
colname=text_to_cstring(column);
2824-
attnum=get_attnum(tableoid,colname);
2825-
if (attnum==InvalidAttrNumber)
2826-
ereport(ERROR,
2827-
(errcode(ERRCODE_UNDEFINED_COLUMN),
2828-
errmsg("column \"%s\" of relation \"%s\" does not exist",
2829-
colname,get_rel_name(tableoid))));
2836+
2837+
/*
2838+
* We don't use get_attnum() here because it will report that dropped
2839+
* columns don't exist. We need to treat dropped columns differently from
2840+
* nonexistent columns.
2841+
*/
2842+
attTuple=SearchSysCache2(ATTNAME,
2843+
ObjectIdGetDatum(tableoid),
2844+
CStringGetDatum(colname));
2845+
if (HeapTupleIsValid(attTuple))
2846+
{
2847+
Form_pg_attributeattributeForm;
2848+
2849+
attributeForm= (Form_pg_attribute)GETSTRUCT(attTuple);
2850+
/* We want to return NULL for dropped columns */
2851+
if (attributeForm->attisdropped)
2852+
attnum=InvalidAttrNumber;
2853+
else
2854+
attnum=attributeForm->attnum;
2855+
ReleaseSysCache(attTuple);
2856+
}
2857+
else
2858+
{
2859+
char*tablename=get_rel_name(tableoid);
2860+
2861+
/*
2862+
* If the table OID is bogus, or it's just been dropped, we'll get
2863+
* NULL back. In such cases we want has_column_privilege to return
2864+
* NULL too, so just return InvalidAttrNumber.
2865+
*/
2866+
if (tablename!=NULL)
2867+
{
2868+
/* tableoid exists, colname does not, so throw error */
2869+
ereport(ERROR,
2870+
(errcode(ERRCODE_UNDEFINED_COLUMN),
2871+
errmsg("column \"%s\" of relation \"%s\" does not exist",
2872+
colname,tablename)));
2873+
}
2874+
/* tableoid doesn't exist, so act like attisdropped case */
2875+
attnum=InvalidAttrNumber;
2876+
}
2877+
28302878
pfree(colname);
28312879
returnattnum;
28322880
}
@@ -3130,6 +3178,9 @@ has_foreign_data_wrapper_privilege_name_id(PG_FUNCTION_ARGS)
31303178
roleid=get_role_oid_or_public(NameStr(*username));
31313179
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
31323180

3181+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3182+
PG_RETURN_NULL();
3183+
31333184
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
31343185

31353186
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3153,6 +3204,9 @@ has_foreign_data_wrapper_privilege_id(PG_FUNCTION_ARGS)
31533204
roleid=GetUserId();
31543205
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
31553206

3207+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3208+
PG_RETURN_NULL();
3209+
31563210
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
31573211

31583212
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3197,6 +3251,9 @@ has_foreign_data_wrapper_privilege_id_id(PG_FUNCTION_ARGS)
31973251

31983252
mode=convert_foreign_data_wrapper_priv_string(priv_type_text);
31993253

3254+
if (!SearchSysCacheExists1(FOREIGNDATAWRAPPEROID,ObjectIdGetDatum(fdwid)))
3255+
PG_RETURN_NULL();
3256+
32003257
aclresult=pg_foreign_data_wrapper_aclcheck(fdwid,roleid,mode);
32013258

32023259
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3896,6 +3953,9 @@ has_server_privilege_name_id(PG_FUNCTION_ARGS)
38963953
roleid=get_role_oid_or_public(NameStr(*username));
38973954
mode=convert_server_priv_string(priv_type_text);
38983955

3956+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
3957+
PG_RETURN_NULL();
3958+
38993959
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39003960

39013961
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3919,6 +3979,9 @@ has_server_privilege_id(PG_FUNCTION_ARGS)
39193979
roleid=GetUserId();
39203980
mode=convert_server_priv_string(priv_type_text);
39213981

3982+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
3983+
PG_RETURN_NULL();
3984+
39223985
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39233986

39243987
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -3963,6 +4026,9 @@ has_server_privilege_id_id(PG_FUNCTION_ARGS)
39634026

39644027
mode=convert_server_priv_string(priv_type_text);
39654028

4029+
if (!SearchSysCacheExists1(FOREIGNSERVEROID,ObjectIdGetDatum(serverid)))
4030+
PG_RETURN_NULL();
4031+
39664032
aclresult=pg_foreign_server_aclcheck(serverid,roleid,mode);
39674033

39684034
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4078,6 +4144,9 @@ has_tablespace_privilege_name_id(PG_FUNCTION_ARGS)
40784144
roleid=get_role_oid_or_public(NameStr(*username));
40794145
mode=convert_tablespace_priv_string(priv_type_text);
40804146

4147+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4148+
PG_RETURN_NULL();
4149+
40814150
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
40824151

40834152
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4101,6 +4170,9 @@ has_tablespace_privilege_id(PG_FUNCTION_ARGS)
41014170
roleid=GetUserId();
41024171
mode=convert_tablespace_priv_string(priv_type_text);
41034172

4173+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4174+
PG_RETURN_NULL();
4175+
41044176
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
41054177

41064178
PG_RETURN_BOOL(aclresult==ACLCHECK_OK);
@@ -4145,6 +4217,9 @@ has_tablespace_privilege_id_id(PG_FUNCTION_ARGS)
41454217

41464218
mode=convert_tablespace_priv_string(priv_type_text);
41474219

4220+
if (!SearchSysCacheExists1(TABLESPACEOID,ObjectIdGetDatum(tablespaceoid)))
4221+
PG_RETURN_NULL();
4222+
41484223
aclresult=pg_tablespace_aclcheck(tablespaceoid,roleid,mode);
41494224

41504225
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
@@ -1098,6 +1098,63 @@ from (select oid from pg_class where relname = 'atest1') as t1;
10981098
f
10991099
(1 row)
11001100

1101+
-- has_column_privilege function
1102+
-- bad-input checks (as non-super-user)
1103+
select has_column_privilege('pg_authid',NULL,'select');
1104+
has_column_privilege
1105+
----------------------
1106+
1107+
(1 row)
1108+
1109+
select has_column_privilege('pg_authid','nosuchcol','select');
1110+
ERROR: column "nosuchcol" of relation "pg_authid" does not exist
1111+
select has_column_privilege(9999,'nosuchcol','select');
1112+
has_column_privilege
1113+
----------------------
1114+
1115+
(1 row)
1116+
1117+
select has_column_privilege(9999,99::int2,'select');
1118+
has_column_privilege
1119+
----------------------
1120+
1121+
(1 row)
1122+
1123+
select has_column_privilege('pg_authid',99::int2,'select');
1124+
has_column_privilege
1125+
----------------------
1126+
1127+
(1 row)
1128+
1129+
select has_column_privilege(9999,99::int2,'select');
1130+
has_column_privilege
1131+
----------------------
1132+
1133+
(1 row)
1134+
1135+
create temp table mytable(f1 int, f2 int, f3 int);
1136+
alter table mytable drop column f2;
1137+
select has_column_privilege('mytable','f2','select');
1138+
ERROR: column "f2" of relation "mytable" does not exist
1139+
select has_column_privilege('mytable','........pg.dropped.2........','select');
1140+
has_column_privilege
1141+
----------------------
1142+
1143+
(1 row)
1144+
1145+
select has_column_privilege('mytable',2::int2,'select');
1146+
has_column_privilege
1147+
----------------------
1148+
t
1149+
(1 row)
1150+
1151+
revoke select on table mytable from regress_user3;
1152+
select has_column_privilege('mytable',2::int2,'select');
1153+
has_column_privilege
1154+
----------------------
1155+
1156+
(1 row)
1157+
11011158
-- Grant options
11021159
SET SESSION AUTHORIZATION regress_user1;
11031160
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
@@ -698,6 +698,23 @@ from (select oid from pg_class where relname = 'atest1') as t1;
698698
select has_table_privilege(t1.oid,'trigger')
699699
from (selectoidfrom pg_classwhere relname='atest1')as t1;
700700

701+
-- has_column_privilege function
702+
703+
-- bad-input checks (as non-super-user)
704+
select has_column_privilege('pg_authid',NULL,'select');
705+
select has_column_privilege('pg_authid','nosuchcol','select');
706+
select has_column_privilege(9999,'nosuchcol','select');
707+
select has_column_privilege(9999,99::int2,'select');
708+
select has_column_privilege('pg_authid',99::int2,'select');
709+
select has_column_privilege(9999,99::int2,'select');
710+
711+
create temp table mytable(f1int, f2int, f3int);
712+
altertable mytable drop column f2;
713+
select has_column_privilege('mytable','f2','select');
714+
select has_column_privilege('mytable','........pg.dropped.2........','select');
715+
select has_column_privilege('mytable',2::int2,'select');
716+
revokeselecton table mytablefrom regress_user3;
717+
select has_column_privilege('mytable',2::int2,'select');
701718

702719
-- Grant options
703720

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp