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

Commit9406884

Browse files
committed
Fix column-privilege leak in error-message paths
While building error messages to return to the user,BuildIndexValueDescription and ri_ReportViolation would happily includethe entire key or entire row in the result returned to the user, even ifthe user didn't have access to view all of the columns being included.Instead, include only those columns which the user is providing or whichthe user has select rights on. If the user does not have any rightsto view the table or any of the columns involved then no detail isprovided and a NULL value is returned from BuildIndexValueDescription.Note that, for key cases, the user must have access to all of thecolumns for the key to be shown; a partial key will not be returned.Back-patch all the way, as column-level privileges are now in allsupported versions.This has been assignedCVE-2014-8161, but since the issue and the patchhave already been publicized on pgsql-hackers, there's no point in tryingto hide this commit.
1 parent8eb1e9d commit9406884

File tree

10 files changed

+215
-35
lines changed

10 files changed

+215
-35
lines changed

‎src/backend/access/index/genam.c‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@
2525
#include"miscadmin.h"
2626
#include"pgstat.h"
2727
#include"storage/bufmgr.h"
28+
#include"utils/acl.h"
2829
#include"utils/builtins.h"
2930
#include"utils/lsyscache.h"
3031
#include"utils/rel.h"
32+
#include"utils/syscache.h"
3133
#include"utils/tqual.h"
3234

3335

@@ -150,6 +152,11 @@ IndexScanEnd(IndexScanDesc scan)
150152
* form "(key_name, ...)=(key_value, ...)". This is currently used
151153
* for building unique-constraint and exclusion-constraint error messages.
152154
*
155+
* Note that if the user does not have permissions to view all of the
156+
* columns involved then a NULL is returned. Returning a partial key seems
157+
* unlikely to be useful and we have no way to know which of the columns the
158+
* user provided.
159+
*
153160
* The passed-in values/nulls arrays are the "raw" input to the index AM,
154161
* e.g. results of FormIndexDatum --- this is not necessarily what is stored
155162
* in the index, but it's what the user perceives to be stored.
@@ -159,13 +166,62 @@ BuildIndexValueDescription(Relation indexRelation,
159166
Datum*values,bool*isnull)
160167
{
161168
StringInfoDatabuf;
169+
Form_pg_indexidxrec;
170+
HeapTupleht_idx;
162171
intnatts=indexRelation->rd_rel->relnatts;
163172
inti;
173+
intkeyno;
174+
Oidindexrelid=RelationGetRelid(indexRelation);
175+
Oidindrelid;
176+
AclResultaclresult;
177+
178+
/*
179+
* Check permissions- if the user does not have access to view all of the
180+
* key columns then return NULL to avoid leaking data.
181+
*
182+
* First we need to check table-level SELECT access and then, if
183+
* there is no access there, check column-level permissions.
184+
*/
185+
186+
/*
187+
* Fetch the pg_index tuple by the Oid of the index
188+
*/
189+
ht_idx=SearchSysCache1(INDEXRELID,ObjectIdGetDatum(indexrelid));
190+
if (!HeapTupleIsValid(ht_idx))
191+
elog(ERROR,"cache lookup failed for index %u",indexrelid);
192+
idxrec= (Form_pg_index)GETSTRUCT(ht_idx);
193+
194+
indrelid=idxrec->indrelid;
195+
Assert(indexrelid==idxrec->indexrelid);
196+
197+
/* Table-level SELECT is enough, if the user has it */
198+
aclresult=pg_class_aclcheck(indrelid,GetUserId(),ACL_SELECT);
199+
if (aclresult!=ACLCHECK_OK)
200+
{
201+
/*
202+
* No table-level access, so step through the columns in the
203+
* index and make sure the user has SELECT rights on all of them.
204+
*/
205+
for (keyno=0;keyno<idxrec->indnatts;keyno++)
206+
{
207+
AttrNumberattnum=idxrec->indkey.values[keyno];
208+
209+
aclresult=pg_attribute_aclcheck(indrelid,attnum,GetUserId(),
210+
ACL_SELECT);
211+
212+
if (aclresult!=ACLCHECK_OK)
213+
{
214+
/* No access, so clean up and return */
215+
ReleaseSysCache(ht_idx);
216+
returnNULL;
217+
}
218+
}
219+
}
220+
ReleaseSysCache(ht_idx);
164221

165222
initStringInfo(&buf);
166223
appendStringInfo(&buf,"(%s)=(",
167-
pg_get_indexdef_columns(RelationGetRelid(indexRelation),
168-
true));
224+
pg_get_indexdef_columns(indexrelid, true));
169225

170226
for (i=0;i<natts;i++)
171227
{

‎src/backend/access/nbtree/nbtinsert.c‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,16 +385,20 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
385385
{
386386
Datumvalues[INDEX_MAX_KEYS];
387387
boolisnull[INDEX_MAX_KEYS];
388+
char*key_desc;
388389

389390
index_deform_tuple(itup,RelationGetDescr(rel),
390391
values,isnull);
392+
393+
key_desc=BuildIndexValueDescription(rel,values,
394+
isnull);
395+
391396
ereport(ERROR,
392397
(errcode(ERRCODE_UNIQUE_VIOLATION),
393398
errmsg("duplicate key value violates unique constraint \"%s\"",
394399
RelationGetRelationName(rel)),
395-
errdetail("Key %s already exists.",
396-
BuildIndexValueDescription(rel,
397-
values,isnull))));
400+
key_desc ?errdetail("Key %s already exists.",
401+
key_desc) :0));
398402
}
399403
}
400404
elseif (all_dead)

‎src/backend/commands/copy.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ typedef struct CopyStateData
148148
Oid*typioparams;/* array of element types for in_functions */
149149
int*defmap;/* array of default att numbers */
150150
ExprState**defexprs;/* array of default att expressions */
151+
List*range_table;
151152

152153
/*
153154
* These variables are used to reduce overhead in textual COPY FROM.
@@ -737,6 +738,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
737738
boolpipe= (stmt->filename==NULL);
738739
Relationrel;
739740
uint64processed;
741+
RangeTblEntry*rte;
740742

741743
/* Disallow file COPY except to superusers. */
742744
if (!pipe&& !superuser())
@@ -750,7 +752,6 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
750752
{
751753
TupleDesctupDesc;
752754
AclModerequired_access= (is_from ?ACL_INSERT :ACL_SELECT);
753-
RangeTblEntry*rte;
754755
List*attnums;
755756
ListCell*cur;
756757

@@ -795,13 +796,15 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
795796

796797
cstate=BeginCopyFrom(rel,stmt->filename,
797798
stmt->attlist,stmt->options);
799+
cstate->range_table=list_make1(rte);
798800
processed=CopyFrom(cstate);/* copy from file to database */
799801
EndCopyFrom(cstate);
800802
}
801803
else
802804
{
803805
cstate=BeginCopyTo(rel,stmt->query,queryString,stmt->filename,
804806
stmt->attlist,stmt->options);
807+
cstate->range_table=list_make1(rte);
805808
processed=DoCopyTo(cstate);/* copy from database to file */
806809
EndCopyTo(cstate);
807810
}
@@ -1933,6 +1936,7 @@ CopyFrom(CopyState cstate)
19331936
estate->es_result_relations=resultRelInfo;
19341937
estate->es_num_result_relations=1;
19351938
estate->es_result_relation_info=resultRelInfo;
1939+
estate->es_range_table=cstate->range_table;
19361940

19371941
/* Set up a tuple slot too */
19381942
myslot=ExecInitExtraTupleSlot(estate);

‎src/backend/commands/trigger.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@
6060
intSessionReplicationRole=SESSION_REPLICATION_ROLE_ORIGIN;
6161

6262

63+
/*
64+
* Note that this macro also exists in executor/execMain.c. There does not
65+
* appear to be any good header to put it into, given the structures that
66+
* it uses, so we let them be duplicated. Be sure to update both if one needs
67+
* to be changed, however.
68+
*/
6369
#defineGetModifiedColumns(relinfo,estate) \
6470
(rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols)
6571

‎src/backend/executor/execMain.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,15 @@ static void intorel_receive(TupleTableSlot *slot, DestReceiver *self);
9595
staticvoidintorel_shutdown(DestReceiver*self);
9696
staticvoidintorel_destroy(DestReceiver*self);
9797

98+
/*
99+
* Note that this macro also exists in commands/trigger.c. There does not
100+
* appear to be any good header to put it into, given the structures that
101+
* it uses, so we let them be duplicated. Be sure to update both if one needs
102+
* to be changed, however.
103+
*/
104+
#defineGetModifiedColumns(relinfo,estate) \
105+
(rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols)
106+
98107
/* end of local decls */
99108

100109

‎src/backend/executor/execUtils.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,15 +1307,19 @@ check_exclusion_constraint(Relation heap, Relation index, IndexInfo *indexInfo,
13071307
(errcode(ERRCODE_EXCLUSION_VIOLATION),
13081308
errmsg("could not create exclusion constraint \"%s\"",
13091309
RelationGetRelationName(index)),
1310-
errdetail("Key %s conflicts with key %s.",
1311-
error_new,error_existing)));
1310+
error_new&&error_existing ?
1311+
errdetail("Key %s conflicts with key %s.",
1312+
error_new,error_existing) :
1313+
errdetail("Key conflicts exist.")));
13121314
else
13131315
ereport(ERROR,
13141316
(errcode(ERRCODE_EXCLUSION_VIOLATION),
13151317
errmsg("conflicting key value violates exclusion constraint \"%s\"",
13161318
RelationGetRelationName(index)),
1317-
errdetail("Key %s conflicts with existing key %s.",
1318-
error_new,error_existing)));
1319+
error_new&&error_existing ?
1320+
errdetail("Key %s conflicts with existing key %s.",
1321+
error_new,error_existing) :
1322+
errdetail("Key conflicts with existing key.")));
13191323
}
13201324

13211325
index_endscan(index_scan);

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

Lines changed: 63 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3495,6 +3495,9 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34953495
boolonfk;
34963496
intidx,
34973497
key_idx;
3498+
Oidrel_oid;
3499+
AclResultaclresult;
3500+
boolhas_perm= true;
34983501

34993502
if (spi_err)
35003503
ereport(ERROR,
@@ -3513,12 +3516,14 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
35133516
if (onfk)
35143517
{
35153518
key_idx=RI_KEYPAIR_FK_IDX;
3519+
rel_oid=fk_rel->rd_id;
35163520
if (tupdesc==NULL)
35173521
tupdesc=fk_rel->rd_att;
35183522
}
35193523
else
35203524
{
35213525
key_idx=RI_KEYPAIR_PK_IDX;
3526+
rel_oid=pk_rel->rd_id;
35223527
if (tupdesc==NULL)
35233528
tupdesc=pk_rel->rd_att;
35243529
}
@@ -3538,45 +3543,81 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
35383543
RelationGetRelationName(pk_rel))));
35393544
}
35403545

3541-
/* Get printable versions of the keys involved */
3542-
initStringInfo(&key_names);
3543-
initStringInfo(&key_values);
3544-
for (idx=0;idx<qkey->nkeypairs;idx++)
3546+
/*
3547+
* Check permissions- if the user does not have access to view the data in
3548+
* any of the key columns then we don't include the errdetail() below.
3549+
*
3550+
* Check table-level permissions first and, failing that, column-level
3551+
* privileges.
3552+
*/
3553+
aclresult=pg_class_aclcheck(rel_oid,GetUserId(),ACL_SELECT);
3554+
if (aclresult!=ACLCHECK_OK)
35453555
{
3546-
intfnum=qkey->keypair[idx][key_idx];
3547-
char*name,
3548-
*val;
3549-
3550-
name=SPI_fname(tupdesc,fnum);
3551-
val=SPI_getvalue(violator,tupdesc,fnum);
3552-
if (!val)
3553-
val="null";
3556+
/* Try for column-level permissions */
3557+
for (idx=0;idx<qkey->nkeypairs;idx++)
3558+
{
3559+
aclresult=pg_attribute_aclcheck(rel_oid,qkey->keypair[idx][key_idx],
3560+
GetUserId(),
3561+
ACL_SELECT);
3562+
/* No access to the key */
3563+
if (aclresult!=ACLCHECK_OK)
3564+
{
3565+
has_perm= false;
3566+
break;
3567+
}
3568+
}
3569+
}
35543570

3555-
if (idx>0)
3571+
if (has_perm)
3572+
{
3573+
/* Get printable versions of the keys involved */
3574+
initStringInfo(&key_names);
3575+
initStringInfo(&key_values);
3576+
for (idx=0;idx<qkey->nkeypairs;idx++)
35563577
{
3557-
appendStringInfoString(&key_names,", ");
3558-
appendStringInfoString(&key_values,", ");
3578+
intfnum=qkey->keypair[idx][key_idx];
3579+
char*name,
3580+
*val;
3581+
3582+
name=SPI_fname(tupdesc,fnum);
3583+
val=SPI_getvalue(violator,tupdesc,fnum);
3584+
if (!val)
3585+
val="null";
3586+
3587+
if (idx>0)
3588+
{
3589+
appendStringInfoString(&key_names,", ");
3590+
appendStringInfoString(&key_values,", ");
3591+
}
3592+
appendStringInfoString(&key_names,name);
3593+
appendStringInfoString(&key_values,val);
35593594
}
3560-
appendStringInfoString(&key_names,name);
3561-
appendStringInfoString(&key_values,val);
35623595
}
35633596

35643597
if (onfk)
35653598
ereport(ERROR,
35663599
(errcode(ERRCODE_FOREIGN_KEY_VIOLATION),
35673600
errmsg("insert or update on table \"%s\" violates foreign key constraint \"%s\"",
3568-
RelationGetRelationName(fk_rel),constrname),
3569-
errdetail("Key (%s)=(%s) is not present in table \"%s\".",
3570-
key_names.data,key_values.data,
3571-
RelationGetRelationName(pk_rel))));
3601+
RelationGetRelationName(fk_rel),
3602+
constrname),
3603+
has_perm ?
3604+
errdetail("Key (%s)=(%s) is not present in table \"%s\".",
3605+
key_names.data,key_values.data,
3606+
RelationGetRelationName(pk_rel)) :
3607+
errdetail("Key is not present in table \"%s\".",
3608+
RelationGetRelationName(pk_rel))));
35723609
else
35733610
ereport(ERROR,
35743611
(errcode(ERRCODE_FOREIGN_KEY_VIOLATION),
35753612
errmsg("update or delete on table \"%s\" violates foreign key constraint \"%s\" on table \"%s\"",
35763613
RelationGetRelationName(pk_rel),
3577-
constrname,RelationGetRelationName(fk_rel)),
3614+
constrname,
3615+
RelationGetRelationName(fk_rel)),
3616+
has_perm ?
35783617
errdetail("Key (%s)=(%s) is still referenced from table \"%s\".",
35793618
key_names.data,key_values.data,
3619+
RelationGetRelationName(fk_rel)) :
3620+
errdetail("Key is still referenced from table \"%s\".",
35803621
RelationGetRelationName(fk_rel))));
35813622
}
35823623

‎src/backend/utils/sort/tuplesort.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3124,15 +3124,18 @@ comparetup_index_btree(const SortTuple *a, const SortTuple *b,
31243124
{
31253125
Datumvalues[INDEX_MAX_KEYS];
31263126
boolisnull[INDEX_MAX_KEYS];
3127+
char*key_desc;
31273128

31283129
index_deform_tuple(tuple1,tupDes,values,isnull);
3130+
3131+
key_desc=BuildIndexValueDescription(state->indexRel,values,isnull);
3132+
31293133
ereport(ERROR,
31303134
(errcode(ERRCODE_UNIQUE_VIOLATION),
31313135
errmsg("could not create unique index \"%s\"",
31323136
RelationGetRelationName(state->indexRel)),
3133-
errdetail("Key %s is duplicated.",
3134-
BuildIndexValueDescription(state->indexRel,
3135-
values,isnull))));
3137+
key_desc ?errdetail("Key %s is duplicated.",key_desc) :
3138+
errdetail("Duplicate keys exist.")));
31363139
}
31373140

31383141
/*

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,34 @@ SELECT atest6 FROM atest6; -- ok
362362
(0 rows)
363363

364364
COPY atest6 TO stdout; -- ok
365+
-- check error reporting with column privs
366+
SET SESSION AUTHORIZATION regressuser1;
367+
CREATE TABLE t1 (c1 int, c2 int, c3 int check (c3 < 5), primary key (c1, c2));
368+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t1_pkey" for table "t1"
369+
GRANT SELECT (c1) ON t1 TO regressuser2;
370+
GRANT INSERT (c1, c2, c3) ON t1 TO regressuser2;
371+
GRANT UPDATE (c1, c2, c3) ON t1 TO regressuser2;
372+
-- seed data
373+
INSERT INTO t1 VALUES (1, 1, 1);
374+
INSERT INTO t1 VALUES (1, 2, 1);
375+
INSERT INTO t1 VALUES (2, 1, 2);
376+
INSERT INTO t1 VALUES (2, 2, 2);
377+
INSERT INTO t1 VALUES (3, 1, 3);
378+
SET SESSION AUTHORIZATION regressuser2;
379+
INSERT INTO t1 (c1, c2) VALUES (1, 1); -- fail, but row not shown
380+
ERROR: duplicate key value violates unique constraint "t1_pkey"
381+
UPDATE t1 SET c2 = 1; -- fail, but row not shown
382+
ERROR: duplicate key value violates unique constraint "t1_pkey"
383+
INSERT INTO t1 (c1, c2) VALUES (null, null); -- fail, but see columns being inserted
384+
ERROR: null value in column "c1" violates not-null constraint
385+
INSERT INTO t1 (c3) VALUES (null); -- fail, but see columns being inserted or have SELECT
386+
ERROR: null value in column "c1" violates not-null constraint
387+
INSERT INTO t1 (c1) VALUES (5); -- fail, but see columns being inserted or have SELECT
388+
ERROR: null value in column "c2" violates not-null constraint
389+
UPDATE t1 SET c3 = 10; -- fail, but see columns with SELECT rights, or being modified
390+
ERROR: new row for relation "t1" violates check constraint "t1_c3_check"
391+
SET SESSION AUTHORIZATION regressuser1;
392+
DROP TABLE t1;
365393
-- test column-level privileges when involved with DELETE
366394
SET SESSION AUTHORIZATION regressuser1;
367395
ALTER TABLE atest6 ADD COLUMN three integer;

0 commit comments

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