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

Commit3a20633

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 parent8c418fb commit3a20633

File tree

10 files changed

+218
-34
lines changed

10 files changed

+218
-34
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

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

166223
initStringInfo(&buf);
167224
appendStringInfo(&buf,"(%s)=(",
168-
pg_get_indexdef_columns(RelationGetRelid(indexRelation),
169-
true));
225+
pg_get_indexdef_columns(indexrelid, true));
170226

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

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,16 +376,20 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
376376
{
377377
Datumvalues[INDEX_MAX_KEYS];
378378
boolisnull[INDEX_MAX_KEYS];
379+
char*key_desc;
379380

380381
index_deform_tuple(itup,RelationGetDescr(rel),
381382
values,isnull);
383+
384+
key_desc=BuildIndexValueDescription(rel,values,
385+
isnull);
386+
382387
ereport(ERROR,
383388
(errcode(ERRCODE_UNIQUE_VIOLATION),
384389
errmsg("duplicate key value violates unique constraint \"%s\"",
385390
RelationGetRelationName(rel)),
386-
errdetail("Key %s already exists.",
387-
BuildIndexValueDescription(rel,
388-
values,isnull))));
391+
key_desc ?errdetail("Key %s already exists.",
392+
key_desc) :0));
389393
}
390394
}
391395
elseif (all_dead)

‎src/backend/commands/copy.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,6 +1687,7 @@ CopyFrom(CopyState cstate)
16871687
booldone= false;
16881688
boolisnull;
16891689
ResultRelInfo*resultRelInfo;
1690+
RangeTblEntry*rte;
16901691
EState*estate=CreateExecutorState();/* for ExecConstraints() */
16911692
TupleTableSlot*slot;
16921693
boolfile_has_oids;
@@ -1807,9 +1808,16 @@ CopyFrom(CopyState cstate)
18071808

18081809
ExecOpenIndices(resultRelInfo);
18091810

1811+
/* Build an RTE to make into a RangeTbl for estate */
1812+
rte=makeNode(RangeTblEntry);
1813+
rte->rtekind=RTE_RELATION;
1814+
rte->relid=RelationGetRelid(cstate->rel);
1815+
rte->requiredPerms=ACL_INSERT;
1816+
18101817
estate->es_result_relations=resultRelInfo;
18111818
estate->es_num_result_relations=1;
18121819
estate->es_result_relation_info=resultRelInfo;
1820+
estate->es_range_table=list_make1(rte);
18131821

18141822
/* Set up a tuple slot too */
18151823
slot=ExecInitExtraTupleSlot(estate);

‎src/backend/commands/trigger.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@
5858
intSessionReplicationRole=SESSION_REPLICATION_ROLE_ORIGIN;
5959

6060

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

‎src/backend/executor/execMain.c

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

88+
/*
89+
* Note that this macro also exists in commands/trigger.c. There does not
90+
* appear to be any good header to put it into, given the structures that
91+
* it uses, so we let them be duplicated. Be sure to update both if one needs
92+
* to be changed, however.
93+
*/
94+
#defineGetModifiedColumns(relinfo,estate) \
95+
(rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols)
96+
8897
/* end of local decls */
8998

9099

‎src/backend/executor/execUtils.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,15 +1299,19 @@ check_exclusion_constraint(Relation heap, Relation index, IndexInfo *indexInfo,
12991299
(errcode(ERRCODE_EXCLUSION_VIOLATION),
13001300
errmsg("could not create exclusion constraint \"%s\"",
13011301
RelationGetRelationName(index)),
1302-
errdetail("Key %s conflicts with key %s.",
1303-
error_new,error_existing)));
1302+
error_new&&error_existing ?
1303+
errdetail("Key %s conflicts with key %s.",
1304+
error_new,error_existing) :
1305+
errdetail("Key conflicts exist.")));
13041306
else
13051307
ereport(ERROR,
13061308
(errcode(ERRCODE_EXCLUSION_VIOLATION),
13071309
errmsg("conflicting key value violates exclusion constraint \"%s\"",
13081310
RelationGetRelationName(index)),
1309-
errdetail("Key %s conflicts with existing key %s.",
1310-
error_new,error_existing)));
1311+
error_new&&error_existing ?
1312+
errdetail("Key %s conflicts with existing key %s.",
1313+
error_new,error_existing) :
1314+
errdetail("Key conflicts with existing key.")));
13111315
}
13121316

13131317
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
@@ -3414,6 +3414,9 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34143414
boolonfk;
34153415
intidx,
34163416
key_idx;
3417+
Oidrel_oid;
3418+
AclResultaclresult;
3419+
boolhas_perm= true;
34173420

34183421
if (spi_err)
34193422
ereport(ERROR,
@@ -3432,12 +3435,14 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34323435
if (onfk)
34333436
{
34343437
key_idx=RI_KEYPAIR_FK_IDX;
3438+
rel_oid=fk_rel->rd_id;
34353439
if (tupdesc==NULL)
34363440
tupdesc=fk_rel->rd_att;
34373441
}
34383442
else
34393443
{
34403444
key_idx=RI_KEYPAIR_PK_IDX;
3445+
rel_oid=pk_rel->rd_id;
34413446
if (tupdesc==NULL)
34423447
tupdesc=pk_rel->rd_att;
34433448
}
@@ -3457,45 +3462,81 @@ ri_ReportViolation(RI_QueryKey *qkey, const char *constrname,
34573462
RelationGetRelationName(pk_rel))));
34583463
}
34593464

3460-
/* Get printable versions of the keys involved */
3461-
initStringInfo(&key_names);
3462-
initStringInfo(&key_values);
3463-
for (idx=0;idx<qkey->nkeypairs;idx++)
3465+
/*
3466+
* Check permissions- if the user does not have access to view the data in
3467+
* any of the key columns then we don't include the errdetail() below.
3468+
*
3469+
* Check table-level permissions first and, failing that, column-level
3470+
* privileges.
3471+
*/
3472+
aclresult=pg_class_aclcheck(rel_oid,GetUserId(),ACL_SELECT);
3473+
if (aclresult!=ACLCHECK_OK)
34643474
{
3465-
intfnum=qkey->keypair[idx][key_idx];
3466-
char*name,
3467-
*val;
3468-
3469-
name=SPI_fname(tupdesc,fnum);
3470-
val=SPI_getvalue(violator,tupdesc,fnum);
3471-
if (!val)
3472-
val="null";
3475+
/* Try for column-level permissions */
3476+
for (idx=0;idx<qkey->nkeypairs;idx++)
3477+
{
3478+
aclresult=pg_attribute_aclcheck(rel_oid,qkey->keypair[idx][key_idx],
3479+
GetUserId(),
3480+
ACL_SELECT);
3481+
/* No access to the key */
3482+
if (aclresult!=ACLCHECK_OK)
3483+
{
3484+
has_perm= false;
3485+
break;
3486+
}
3487+
}
3488+
}
34733489

3474-
if (idx>0)
3490+
if (has_perm)
3491+
{
3492+
/* Get printable versions of the keys involved */
3493+
initStringInfo(&key_names);
3494+
initStringInfo(&key_values);
3495+
for (idx=0;idx<qkey->nkeypairs;idx++)
34753496
{
3476-
appendStringInfoString(&key_names,", ");
3477-
appendStringInfoString(&key_values,", ");
3497+
intfnum=qkey->keypair[idx][key_idx];
3498+
char*name,
3499+
*val;
3500+
3501+
name=SPI_fname(tupdesc,fnum);
3502+
val=SPI_getvalue(violator,tupdesc,fnum);
3503+
if (!val)
3504+
val="null";
3505+
3506+
if (idx>0)
3507+
{
3508+
appendStringInfoString(&key_names,", ");
3509+
appendStringInfoString(&key_values,", ");
3510+
}
3511+
appendStringInfoString(&key_names,name);
3512+
appendStringInfoString(&key_values,val);
34783513
}
3479-
appendStringInfoString(&key_names,name);
3480-
appendStringInfoString(&key_values,val);
34813514
}
34823515

34833516
if (onfk)
34843517
ereport(ERROR,
34853518
(errcode(ERRCODE_FOREIGN_KEY_VIOLATION),
34863519
errmsg("insert or update on table \"%s\" violates foreign key constraint \"%s\"",
3487-
RelationGetRelationName(fk_rel),constrname),
3488-
errdetail("Key (%s)=(%s) is not present in table \"%s\".",
3489-
key_names.data,key_values.data,
3490-
RelationGetRelationName(pk_rel))));
3520+
RelationGetRelationName(fk_rel),
3521+
constrname),
3522+
has_perm ?
3523+
errdetail("Key (%s)=(%s) is not present in table \"%s\".",
3524+
key_names.data,key_values.data,
3525+
RelationGetRelationName(pk_rel)) :
3526+
errdetail("Key is not present in table \"%s\".",
3527+
RelationGetRelationName(pk_rel))));
34913528
else
34923529
ereport(ERROR,
34933530
(errcode(ERRCODE_FOREIGN_KEY_VIOLATION),
34943531
errmsg("update or delete on table \"%s\" violates foreign key constraint \"%s\" on table \"%s\"",
34953532
RelationGetRelationName(pk_rel),
3496-
constrname,RelationGetRelationName(fk_rel)),
3533+
constrname,
3534+
RelationGetRelationName(fk_rel)),
3535+
has_perm ?
34973536
errdetail("Key (%s)=(%s) is still referenced from table \"%s\".",
34983537
key_names.data,key_values.data,
3538+
RelationGetRelationName(fk_rel)) :
3539+
errdetail("Key is still referenced from table \"%s\".",
34993540
RelationGetRelationName(fk_rel))));
35003541
}
35013542

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2799,15 +2799,18 @@ comparetup_index_btree(const SortTuple *a, const SortTuple *b,
27992799
{
28002800
Datumvalues[INDEX_MAX_KEYS];
28012801
boolisnull[INDEX_MAX_KEYS];
2802+
char*key_desc;
28022803

28032804
index_deform_tuple(tuple1,tupDes,values,isnull);
2805+
2806+
key_desc=BuildIndexValueDescription(state->indexRel,values,isnull);
2807+
28042808
ereport(ERROR,
28052809
(errcode(ERRCODE_UNIQUE_VIOLATION),
28062810
errmsg("could not create unique index \"%s\"",
28072811
RelationGetRelationName(state->indexRel)),
2808-
errdetail("Key %s is duplicated.",
2809-
BuildIndexValueDescription(state->indexRel,
2810-
values,isnull))));
2812+
key_desc ?errdetail("Key %s is duplicated.",key_desc) :
2813+
errdetail("Duplicate keys exist.")));
28112814
}
28122815

28132816
/*

‎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