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

Commit3cc74a3

Browse files
committed
Fix column-privilege leak in error-message paths
While building error messages to return to the user,BuildIndexValueDescription, ExecBuildSlotValueDescription andri_ReportViolation would happily include the entire key or entire row inthe result returned to the user, even if the user didn't have access toview 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 BuildIndexValueDescriptionand ExecBuildSlotValueDescription. Note that, for key cases, the usermust have access to all of the columns for the key to be shown; apartial 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 parent3d5e857 commit3cc74a3

File tree

11 files changed

+361
-70
lines changed

11 files changed

+361
-70
lines changed

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

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
#include"lib/stringinfo.h"
2626
#include"miscadmin.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"
3132
#include"utils/snapmgr.h"
33+
#include"utils/syscache.h"
3234
#include"utils/tqual.h"
3335

3436

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

169226
initStringInfo(&buf);
170227
appendStringInfo(&buf,"(%s)=(",
171-
pg_get_indexdef_columns(RelationGetRelid(indexRelation),
172-
true));
228+
pg_get_indexdef_columns(indexrelid, true));
173229

174230
for (i=0;i<natts;i++)
175231
{

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,16 +388,20 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,
388388
{
389389
Datumvalues[INDEX_MAX_KEYS];
390390
boolisnull[INDEX_MAX_KEYS];
391+
char*key_desc;
391392

392393
index_deform_tuple(itup,RelationGetDescr(rel),
393394
values,isnull);
395+
396+
key_desc=BuildIndexValueDescription(rel,values,
397+
isnull);
398+
394399
ereport(ERROR,
395400
(errcode(ERRCODE_UNIQUE_VIOLATION),
396401
errmsg("duplicate key value violates unique constraint \"%s\"",
397402
RelationGetRelationName(rel)),
398-
errdetail("Key %s already exists.",
399-
BuildIndexValueDescription(rel,
400-
values,isnull)),
403+
key_desc ?errdetail("Key %s already exists.",
404+
key_desc) :0,
401405
errtableconstraint(heapRel,
402406
RelationGetRelationName(rel))));
403407
}

‎src/backend/commands/copy.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ typedef struct CopyStateData
160160
int*defmap;/* array of default att numbers */
161161
ExprState**defexprs;/* array of default att expressions */
162162
boolvolatile_defexprs;/* is any of defexprs volatile? */
163+
List*range_table;
163164

164165
/*
165166
* These variables are used to reduce overhead in textual COPY FROM.
@@ -784,6 +785,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
784785
boolpipe= (stmt->filename==NULL);
785786
Relationrel;
786787
Oidrelid;
788+
RangeTblEntry*rte;
787789

788790
/* Disallow COPY to/from file or program except to superusers. */
789791
if (!pipe&& !superuser())
@@ -806,7 +808,6 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
806808
{
807809
TupleDesctupDesc;
808810
AclModerequired_access= (is_from ?ACL_INSERT :ACL_SELECT);
809-
RangeTblEntry*rte;
810811
List*attnums;
811812
ListCell*cur;
812813

@@ -856,6 +857,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
856857

857858
cstate=BeginCopyFrom(rel,stmt->filename,stmt->is_program,
858859
stmt->attlist,stmt->options);
860+
cstate->range_table=list_make1(rte);
859861
*processed=CopyFrom(cstate);/* copy from file to database */
860862
EndCopyFrom(cstate);
861863
}
@@ -864,6 +866,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
864866
cstate=BeginCopyTo(rel,stmt->query,queryString,
865867
stmt->filename,stmt->is_program,
866868
stmt->attlist,stmt->options);
869+
cstate->range_table=list_make1(rte);
867870
*processed=DoCopyTo(cstate);/* copy from database to file */
868871
EndCopyTo(cstate);
869872
}
@@ -2184,6 +2187,7 @@ CopyFrom(CopyState cstate)
21842187
estate->es_result_relations=resultRelInfo;
21852188
estate->es_num_result_relations=1;
21862189
estate->es_result_relation_info=resultRelInfo;
2190+
estate->es_range_table=cstate->range_table;
21872191

21882192
/* Set up a tuple slot too */
21892193
myslot=ExecInitExtraTupleSlot(estate);

‎src/backend/commands/matview.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,13 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
586586
elog(ERROR,"SPI_exec failed: %s",querybuf.data);
587587
if (SPI_processed>0)
588588
{
589+
/*
590+
* Note that this ereport() is returning data to the user. Generally,
591+
* we would want to make sure that the user has been granted access to
592+
* this data. However, REFRESH MAT VIEW is only able to be run by the
593+
* owner of the mat view (or a superuser) and therefore there is no
594+
* need to check for access to data in the mat view.
595+
*/
589596
ereport(ERROR,
590597
(errcode(ERRCODE_CARDINALITY_VIOLATION),
591598
errmsg("new data for \"%s\" contains duplicate rows without any null columns",

‎src/backend/commands/trigger.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@ intSessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN;
6565
/* How many levels deep into trigger execution are we? */
6666
staticintMyTriggerDepth=0;
6767

68+
/*
69+
* Note that this macro also exists in executor/execMain.c. There does not
70+
* appear to be any good header to put it into, given the structures that
71+
* it uses, so we let them be duplicated. Be sure to update both if one needs
72+
* to be changed, however.
73+
*/
6874
#defineGetModifiedColumns(relinfo,estate) \
6975
(rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols)
7076

0 commit comments

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