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

Commitae20b23

Browse files
committed
Refactor permissions checks for large objects.
Up to now, ACL checks for large objects happened at the level ofthe SQL-callable functions, which led toCVE-2017-7548 because of amissing check. Push them down to be enforced in inv_api.c as muchas possible, in hopes of preventing future bugs. This does have theeffect of moving read and write permission errors to happen at lo_opentime not loread or lowrite time, but that seems acceptable.Michael Paquier and Tom LaneDiscussion:https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com
1 parent5ecc0d7 commitae20b23

File tree

6 files changed

+117
-111
lines changed

6 files changed

+117
-111
lines changed

‎src/backend/catalog/objectaddress.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,13 @@
6969
#include"commands/trigger.h"
7070
#include"foreign/foreign.h"
7171
#include"funcapi.h"
72-
#include"libpq/be-fsstubs.h"
7372
#include"miscadmin.h"
7473
#include"nodes/makefuncs.h"
7574
#include"parser/parse_func.h"
7675
#include"parser/parse_oper.h"
7776
#include"parser/parse_type.h"
7877
#include"rewrite/rewriteSupport.h"
78+
#include"storage/large_object.h"
7979
#include"storage/lmgr.h"
8080
#include"storage/sinval.h"
8181
#include"utils/builtins.h"

‎src/backend/libpq/be-fsstubs.c

Lines changed: 17 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,6 @@
5151
#include"utils/builtins.h"
5252
#include"utils/memutils.h"
5353

54-
/*
55-
* compatibility flag for permission checks
56-
*/
57-
boollo_compat_privileges;
58-
5954
/* define this to enable debug logging */
6055
/* #define FSDB 1 */
6156
/* chunk size for lo_import/lo_export transfers */
@@ -108,14 +103,6 @@ be_lo_open(PG_FUNCTION_ARGS)
108103

109104
lobjDesc=inv_open(lobjId,mode,fscxt);
110105

111-
if (lobjDesc==NULL)
112-
{/* lookup failed */
113-
#ifFSDB
114-
elog(DEBUG4,"could not open large object %u",lobjId);
115-
#endif
116-
PG_RETURN_INT32(-1);
117-
}
118-
119106
fd=newLOfd(lobjDesc);
120107

121108
PG_RETURN_INT32(fd);
@@ -163,22 +150,16 @@ lo_read(int fd, char *buf, int len)
163150
errmsg("invalid large-object descriptor: %d",fd)));
164151
lobj=cookies[fd];
165152

166-
/* We don't bother to check IFS_RDLOCK, since it's always set */
167-
168-
/* Permission checks --- first time through only */
169-
if ((lobj->flags&IFS_RD_PERM_OK)==0)
170-
{
171-
if (!lo_compat_privileges&&
172-
pg_largeobject_aclcheck_snapshot(lobj->id,
173-
GetUserId(),
174-
ACL_SELECT,
175-
lobj->snapshot)!=ACLCHECK_OK)
176-
ereport(ERROR,
177-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
178-
errmsg("permission denied for large object %u",
179-
lobj->id)));
180-
lobj->flags |=IFS_RD_PERM_OK;
181-
}
153+
/*
154+
* Check state. inv_read() would throw an error anyway, but we want the
155+
* error to be about the FD's state not the underlying privilege; it might
156+
* be that the privilege exists but user forgot to ask for read mode.
157+
*/
158+
if ((lobj->flags&IFS_RDLOCK)==0)
159+
ereport(ERROR,
160+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
161+
errmsg("large object descriptor %d was not opened for reading",
162+
fd)));
182163

183164
status=inv_read(lobj,buf,len);
184165

@@ -197,27 +178,13 @@ lo_write(int fd, const char *buf, int len)
197178
errmsg("invalid large-object descriptor: %d",fd)));
198179
lobj=cookies[fd];
199180

181+
/* see comment in lo_read() */
200182
if ((lobj->flags&IFS_WRLOCK)==0)
201183
ereport(ERROR,
202184
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
203185
errmsg("large object descriptor %d was not opened for writing",
204186
fd)));
205187

206-
/* Permission checks --- first time through only */
207-
if ((lobj->flags&IFS_WR_PERM_OK)==0)
208-
{
209-
if (!lo_compat_privileges&&
210-
pg_largeobject_aclcheck_snapshot(lobj->id,
211-
GetUserId(),
212-
ACL_UPDATE,
213-
lobj->snapshot)!=ACLCHECK_OK)
214-
ereport(ERROR,
215-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
216-
errmsg("permission denied for large object %u",
217-
lobj->id)));
218-
lobj->flags |=IFS_WR_PERM_OK;
219-
}
220-
221188
status=inv_write(lobj,buf,len);
222189

223190
returnstatus;
@@ -342,7 +309,11 @@ be_lo_unlink(PG_FUNCTION_ARGS)
342309
{
343310
OidlobjId=PG_GETARG_OID(0);
344311

345-
/* Must be owner of the largeobject */
312+
/*
313+
* Must be owner of the large object. It would be cleaner to check this
314+
* in inv_drop(), but we want to throw the error before not after closing
315+
* relevant FDs.
316+
*/
346317
if (!lo_compat_privileges&&
347318
!pg_largeobject_ownercheck(lobjId,GetUserId()))
348319
ereport(ERROR,
@@ -574,27 +545,13 @@ lo_truncate_internal(int32 fd, int64 len)
574545
errmsg("invalid large-object descriptor: %d",fd)));
575546
lobj=cookies[fd];
576547

548+
/* see comment in lo_read() */
577549
if ((lobj->flags&IFS_WRLOCK)==0)
578550
ereport(ERROR,
579551
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
580552
errmsg("large object descriptor %d was not opened for writing",
581553
fd)));
582554

583-
/* Permission checks --- first time through only */
584-
if ((lobj->flags&IFS_WR_PERM_OK)==0)
585-
{
586-
if (!lo_compat_privileges&&
587-
pg_largeobject_aclcheck_snapshot(lobj->id,
588-
GetUserId(),
589-
ACL_UPDATE,
590-
lobj->snapshot)!=ACLCHECK_OK)
591-
ereport(ERROR,
592-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
593-
errmsg("permission denied for large object %u",
594-
lobj->id)));
595-
lobj->flags |=IFS_WR_PERM_OK;
596-
}
597-
598555
inv_truncate(lobj,len);
599556
}
600557

@@ -770,17 +727,6 @@ lo_get_fragment_internal(Oid loOid, int64 offset, int32 nbytes)
770727

771728
loDesc=inv_open(loOid,INV_READ,fscxt);
772729

773-
/* Permission check */
774-
if (!lo_compat_privileges&&
775-
pg_largeobject_aclcheck_snapshot(loDesc->id,
776-
GetUserId(),
777-
ACL_SELECT,
778-
loDesc->snapshot)!=ACLCHECK_OK)
779-
ereport(ERROR,
780-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
781-
errmsg("permission denied for large object %u",
782-
loDesc->id)));
783-
784730
/*
785731
* Compute number of bytes we'll actually read, accommodating nbytes == -1
786732
* and reads beyond the end of the LO.

‎src/backend/storage/large_object/inv_api.c

Lines changed: 85 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@
5151
#include"utils/tqual.h"
5252

5353

54+
/*
55+
* GUC: backwards-compatibility flag to suppress LO permission checks
56+
*/
57+
boollo_compat_privileges;
58+
5459
/*
5560
* All accesses to pg_largeobject and its index make use of a single Relation
5661
* reference, so that we only need to open pg_relation once per transaction.
@@ -250,46 +255,78 @@ inv_open(Oid lobjId, int flags, MemoryContext mcxt)
250255
Snapshotsnapshot=NULL;
251256
intdescflags=0;
252257

258+
/*
259+
* Historically, no difference is made between (INV_WRITE) and (INV_WRITE
260+
* | INV_READ), the caller being allowed to read the large object
261+
* descriptor in either case.
262+
*/
253263
if (flags&INV_WRITE)
254-
{
255-
snapshot=NULL;/* instantaneous MVCC snapshot */
256-
descflags=IFS_WRLOCK |IFS_RDLOCK;
257-
}
258-
elseif (flags&INV_READ)
259-
{
260-
snapshot=GetActiveSnapshot();
261-
descflags=IFS_RDLOCK;
262-
}
263-
else
264+
descflags |=IFS_WRLOCK |IFS_RDLOCK;
265+
if (flags&INV_READ)
266+
descflags |=IFS_RDLOCK;
267+
268+
if (descflags==0)
264269
ereport(ERROR,
265270
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
266271
errmsg("invalid flags for opening a large object: %d",
267272
flags)));
268273

274+
/* Get snapshot. If write is requested, use an instantaneous snapshot. */
275+
if (descflags&IFS_WRLOCK)
276+
snapshot=NULL;
277+
else
278+
snapshot=GetActiveSnapshot();
279+
269280
/* Can't use LargeObjectExists here because we need to specify snapshot */
270281
if (!myLargeObjectExists(lobjId,snapshot))
271282
ereport(ERROR,
272283
(errcode(ERRCODE_UNDEFINED_OBJECT),
273284
errmsg("large object %u does not exist",lobjId)));
274285

286+
/* Apply permission checks, again specifying snapshot */
287+
if ((descflags&IFS_RDLOCK)!=0)
288+
{
289+
if (!lo_compat_privileges&&
290+
pg_largeobject_aclcheck_snapshot(lobjId,
291+
GetUserId(),
292+
ACL_SELECT,
293+
snapshot)!=ACLCHECK_OK)
294+
ereport(ERROR,
295+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
296+
errmsg("permission denied for large object %u",
297+
lobjId)));
298+
}
299+
if ((descflags&IFS_WRLOCK)!=0)
300+
{
301+
if (!lo_compat_privileges&&
302+
pg_largeobject_aclcheck_snapshot(lobjId,
303+
GetUserId(),
304+
ACL_UPDATE,
305+
snapshot)!=ACLCHECK_OK)
306+
ereport(ERROR,
307+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
308+
errmsg("permission denied for large object %u",
309+
lobjId)));
310+
}
311+
312+
/* OK to create a descriptor */
313+
retval= (LargeObjectDesc*)MemoryContextAlloc(mcxt,
314+
sizeof(LargeObjectDesc));
315+
retval->id=lobjId;
316+
retval->subid=GetCurrentSubTransactionId();
317+
retval->offset=0;
318+
retval->flags=descflags;
319+
275320
/*
276321
* We must register the snapshot in TopTransaction's resowner, because it
277322
* must stay alive until the LO is closed rather than until the current
278-
* portal shuts down. Do thisafter checking that the LO exists, to avoid
279-
*leaking thesnapshot if an error is thrown.
323+
* portal shuts down.Do thislast to avoid uselessly leaking the
324+
* snapshot if an error is thrown above.
280325
*/
281326
if (snapshot)
282327
snapshot=RegisterSnapshotOnOwner(snapshot,
283328
TopTransactionResourceOwner);
284-
285-
/* All set, create a descriptor */
286-
retval= (LargeObjectDesc*)MemoryContextAlloc(mcxt,
287-
sizeof(LargeObjectDesc));
288-
retval->id=lobjId;
289-
retval->subid=GetCurrentSubTransactionId();
290-
retval->offset=0;
291329
retval->snapshot=snapshot;
292-
retval->flags=descflags;
293330

294331
returnretval;
295332
}
@@ -312,7 +349,7 @@ inv_close(LargeObjectDesc *obj_desc)
312349
/*
313350
* Destroys an existing large object (not to be confused with a descriptor!)
314351
*
315-
*returns -1 if failed
352+
*Note we expect caller to have done any required permissions check.
316353
*/
317354
int
318355
inv_drop(OidlobjId)
@@ -333,6 +370,7 @@ inv_drop(Oid lobjId)
333370
*/
334371
CommandCounterIncrement();
335372

373+
/* For historical reasons, we always return 1 on success. */
336374
return1;
337375
}
338376

@@ -397,6 +435,11 @@ inv_seek(LargeObjectDesc *obj_desc, int64 offset, int whence)
397435

398436
Assert(PointerIsValid(obj_desc));
399437

438+
/*
439+
* We allow seek/tell if you have either read or write permission, so no
440+
* need for a permission check here.
441+
*/
442+
400443
/*
401444
* Note: overflow in the additions is possible, but since we will reject
402445
* negative results, we don't need any extra test for that.
@@ -439,6 +482,11 @@ inv_tell(LargeObjectDesc *obj_desc)
439482
{
440483
Assert(PointerIsValid(obj_desc));
441484

485+
/*
486+
* We allow seek/tell if you have either read or write permission, so no
487+
* need for a permission check here.
488+
*/
489+
442490
returnobj_desc->offset;
443491
}
444492

@@ -458,6 +506,12 @@ inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes)
458506
Assert(PointerIsValid(obj_desc));
459507
Assert(buf!=NULL);
460508

509+
if ((obj_desc->flags&IFS_RDLOCK)==0)
510+
ereport(ERROR,
511+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
512+
errmsg("permission denied for large object %u",
513+
obj_desc->id)));
514+
461515
if (nbytes <=0)
462516
return0;
463517

@@ -563,7 +617,11 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
563617
Assert(buf!=NULL);
564618

565619
/* enforce writability because snapshot is probably wrong otherwise */
566-
Assert(obj_desc->flags&IFS_WRLOCK);
620+
if ((obj_desc->flags&IFS_WRLOCK)==0)
621+
ereport(ERROR,
622+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
623+
errmsg("permission denied for large object %u",
624+
obj_desc->id)));
567625

568626
if (nbytes <=0)
569627
return0;
@@ -749,7 +807,11 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len)
749807
Assert(PointerIsValid(obj_desc));
750808

751809
/* enforce writability because snapshot is probably wrong otherwise */
752-
Assert(obj_desc->flags&IFS_WRLOCK);
810+
if ((obj_desc->flags&IFS_WRLOCK)==0)
811+
ereport(ERROR,
812+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
813+
errmsg("permission denied for large object %u",
814+
obj_desc->id)));
753815

754816
/*
755817
* use errmsg_internal here because we don't want to expose INT64_FORMAT

0 commit comments

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