Hi all,
Recent commit 8d98819 has added a missing permissoin check to lo_put()
to make sure that the write permissions of the object are properly set
before writing to a large object. When studying the problem, we bumped
into the fact that it is possible to actually simplify those ACL
checks and replace them by checks when opening the large object. This
makes all the checks now in be-fsstubs.c happen in inv_api.c, which is
where all the large object handling happens at a lower level. This
way, it is also possible to remove the extra logic in place to have
the permission checks happen only once.
At the same time, we have bumped into two things:
- ALLOW_DANGEROUS_LO_FUNCTIONS has been introduced in 1999, so it
could be time to let it go. I have known of no place where this is
actively used.
- lo_import and lo_export on the backend have superuser checks. We
could remove them and replace them with proper REVOKE permissions to
allow administrators to give access to those functions.
Attached is a set of 3 patches:
- 0001 removes ALLOW_DANGEROUS_LO_FUNCTIONS
- 0002 replaces the superuser checks with GRANT permissions
- 0003 refactors the LO code to improve the ACL handling. Note that
this patch introduces a more debatable change: now there is no
distinction between INV_WRITE and INV_WRITE|INV_READ, as when
INV_WRITE is used INV_READ is implied. The patch as proposed does a
complete split of both permissions to give a system more natural and
more flexible. The current behavior is documented. Please note as well
that it is possible to implement a patch that keeps compatibility as
well, but I would welcome a debate on the matter. This patch owns also
a good deal to Tom.
I am parking them to the next commit fest for PG11.
Thanks,
--
Michael
From 2a2b2beddc5b01ffe6de7e442e20e00b4e518859 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 14 Aug 2017 12:01:58 +0900
Subject: [PATCH 3/3] Move ACL checks for large objects when opening them
Up to now, ACL checks for large objects happened at the the level of
the SQL-callable functions, which led to CVE-2017-7548 because of a
missing check. It is actually possible to simplify those checks when
opening a large object, which simplifies the code checking them so as
there is no need to track if a check has already been done or not and
this removes any risk of missing again permission checks if a function
related to large objects is added in the future.
A more debatable change that this patch introduces is that opening
a large object for write is equivalent to read-write, which is a
behavior documented, into a complete split of the checks. This gives
the user more flexibility at the risk of breaking some applications,
and makes the new behavior more natural.
Patch by Tom Lane and Michael Paquier.
---
doc/src/sgml/lobj.sgml | 25 +++----
src/backend/catalog/objectaddress.c | 2 +-
src/backend/libpq/be-fsstubs.c | 88 +++++-------------------
src/backend/storage/large_object/inv_api.c | 103 ++++++++++++++++++++++-------
src/backend/utils/misc/guc.c | 2 +-
src/include/libpq/be-fsstubs.h | 5 --
src/include/storage/large_object.h | 13 ++--
src/test/regress/expected/privileges.out | 8 +--
src/test/regress/sql/privileges.sql | 2 +-
9 files changed, 121 insertions(+), 127 deletions(-)
diff --git a/doc/src/sgml/lobj.sgml b/doc/src/sgml/lobj.sgml
index 7757e1e441..2025545c75 100644
--- a/doc/src/sgml/lobj.sgml
+++ b/doc/src/sgml/lobj.sgml
@@ -276,20 +276,17 @@ int lo_open(PGconn *conn, Oid lobjId, int mode);
</para>
<para>
- The server currently does not distinguish between modes
- <symbol>INV_WRITE</symbol> and <symbol>INV_READ</> <literal>|</>
- <symbol>INV_WRITE</symbol>: you are allowed to read from the descriptor
- in either case. However there is a significant difference between
- these modes and <symbol>INV_READ</> alone: with <symbol>INV_READ</>
- you cannot write on the descriptor, and the data read from it will
- reflect the contents of the large object at the time of the transaction
- snapshot that was active when <function>lo_open</> was executed,
- regardless of later writes by this or other transactions. Reading
- from a descriptor opened with <symbol>INV_WRITE</symbol> returns
- data that reflects all writes of other committed transactions as well
- as writes of the current transaction. This is similar to the behavior
- of <literal>REPEATABLE READ</> versus <literal>READ COMMITTED</> transaction
- modes for ordinary SQL <command>SELECT</> commands.
+ With only <symbol>INV_READ</> you cannot write on the descriptor,
+ and the data read from it will reflect the contents of the large object
+ at the time of the transaction snapshot that was active when
+ <function>lo_open</> was executed, regardless of later writes by this
+ or other transactions. Reading from a descriptor opened with
+ <symbol>INV_WRITE</symbol> or <symbol>INV_READ</> <literal>|</>
+ <symbol>INV_WRITE</symbol> returns data that reflects all writes of
+ other committed transactions as well as writes of the current
+ transaction. This is similar to the behavior of
+ <literal>REPEATABLE READ</> versus <literal>READ COMMITTED</>
+ transaction modes for ordinary SQL <command>SELECT</> commands.
</para>
<para>
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 6cac2dfd1d..ded4b38f31 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -69,13 +69,13 @@
#include "commands/trigger.h"
#include "foreign/foreign.h"
#include "funcapi.h"
-#include "libpq/be-fsstubs.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "parser/parse_func.h"
#include "parser/parse_oper.h"
#include "parser/parse_type.h"
#include "rewrite/rewriteSupport.h"
+#include "storage/large_object.h"
#include "storage/lmgr.h"
#include "storage/sinval.h"
#include "utils/builtins.h"
diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 507843c386..60763ff1a5 100644
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -51,11 +51,6 @@
#include "utils/builtins.h"
#include "utils/memutils.h"
-/*
- * compatibility flag for permission checks
- */
-bool lo_compat_privileges;
-
/* define this to enable debug logging */
/* #define FSDB 1 */
/* chunk size for lo_import/lo_export transfers */
@@ -108,14 +103,6 @@ be_lo_open(PG_FUNCTION_ARGS)
lobjDesc = inv_open(lobjId, mode, fscxt);
- if (lobjDesc == NULL)
- { /* lookup failed */
-#if FSDB
- elog(DEBUG4, "could not open large object %u", lobjId);
-#endif
- PG_RETURN_INT32(-1);
- }
-
fd = newLOfd(lobjDesc);
PG_RETURN_INT32(fd);
@@ -163,22 +150,16 @@ lo_read(int fd, char *buf, int len)
errmsg("invalid large-object descriptor: %d", fd)));
lobj = cookies[fd];
- /* We don't bother to check IFS_RDLOCK, since it's always set */
-
- /* Permission checks --- first time through only */
- if ((lobj->flags & IFS_RD_PERM_OK) == 0)
- {
- if (!lo_compat_privileges &&
- pg_largeobject_aclcheck_snapshot(lobj->id,
- GetUserId(),
- ACL_SELECT,
- lobj->snapshot) != ACLCHECK_OK)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied for large object %u",
- lobj->id)));
- lobj->flags |= IFS_RD_PERM_OK;
- }
+ /*
+ * Check state. inv_read() would throw an error anyway, but we want the
+ * error to be about the FD's state not the underlying privilege; it might
+ * be that the privilege exists but user forgot to ask for read mode.
+ */
+ if ((lobj->flags & IFS_RDLOCK) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("large object descriptor %d was not opened for reading",
+ fd)));
status = inv_read(lobj, buf, len);
@@ -197,27 +178,13 @@ lo_write(int fd, const char *buf, int len)
errmsg("invalid large-object descriptor: %d", fd)));
lobj = cookies[fd];
+ /* see comment in lo_read() */
if ((lobj->flags & IFS_WRLOCK) == 0)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("large object descriptor %d was not opened for writing",
fd)));
- /* Permission checks --- first time through only */
- if ((lobj->flags & IFS_WR_PERM_OK) == 0)
- {
- if (!lo_compat_privileges &&
- pg_largeobject_aclcheck_snapshot(lobj->id,
- GetUserId(),
- ACL_UPDATE,
- lobj->snapshot) != ACLCHECK_OK)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied for large object %u",
- lobj->id)));
- lobj->flags |= IFS_WR_PERM_OK;
- }
-
status = inv_write(lobj, buf, len);
return status;
@@ -342,7 +309,11 @@ be_lo_unlink(PG_FUNCTION_ARGS)
{
Oid lobjId = PG_GETARG_OID(0);
- /* Must be owner of the largeobject */
+ /*
+ * Must be owner of the large object. It would be cleaner to check this
+ * in inv_drop(), but we want to throw the error before not after closing
+ * relevant FDs.
+ */
if (!lo_compat_privileges &&
!pg_largeobject_ownercheck(lobjId, GetUserId()))
ereport(ERROR,
@@ -565,27 +536,13 @@ lo_truncate_internal(int32 fd, int64 len)
errmsg("invalid large-object descriptor: %d", fd)));
lobj = cookies[fd];
+ /* see comment in lo_read() */
if ((lobj->flags & IFS_WRLOCK) == 0)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("large object descriptor %d was not opened for writing",
fd)));
- /* Permission checks --- first time through only */
- if ((lobj->flags & IFS_WR_PERM_OK) == 0)
- {
- if (!lo_compat_privileges &&
- pg_largeobject_aclcheck_snapshot(lobj->id,
- GetUserId(),
- ACL_UPDATE,
- lobj->snapshot) != ACLCHECK_OK)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied for large object %u",
- lobj->id)));
- lobj->flags |= IFS_WR_PERM_OK;
- }
-
inv_truncate(lobj, len);
}
@@ -761,17 +718,6 @@ lo_get_fragment_internal(Oid loOid, int64 offset, int32 nbytes)
loDesc = inv_open(loOid, INV_READ, fscxt);
- /* Permission check */
- if (!lo_compat_privileges &&
- pg_largeobject_aclcheck_snapshot(loDesc->id,
- GetUserId(),
- ACL_SELECT,
- loDesc->snapshot) != ACLCHECK_OK)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied for large object %u",
- loDesc->id)));
-
/*
* Compute number of bytes we'll actually read, accommodating nbytes == -1
* and reads beyond the end of the LO.
diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c
index d55d40e6f8..495b44fa93 100644
--- a/src/backend/storage/large_object/inv_api.c
+++ b/src/backend/storage/large_object/inv_api.c
@@ -51,6 +51,11 @@
#include "utils/tqual.h"
+/*
+ * GUC: backwards-compatibility flag to suppress LO permission checks
+ */
+bool lo_compat_privileges;
+
/*
* All accesses to pg_largeobject and its index make use of a single Relation
* reference, so that we only need to open pg_relation once per transaction.
@@ -269,45 +274,72 @@ inv_open(Oid lobjId, int flags, MemoryContext mcxt)
int descflags = 0;
if (flags & INV_WRITE)
- {
- snapshot = NULL; /* instantaneous MVCC snapshot */
- descflags = IFS_WRLOCK | IFS_RDLOCK;
- }
- else if (flags & INV_READ)
- {
- snapshot = GetActiveSnapshot();
- descflags = IFS_RDLOCK;
- }
- else
+ descflags |= IFS_WRLOCK;
+ if (flags & INV_READ)
+ descflags |= IFS_RDLOCK;
+
+ if (descflags == 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid flags for opening a large object: %d",
flags)));
+ /* Get snapshot. If write is requested, use an instantaneous snapshot. */
+ if (descflags & IFS_WRLOCK)
+ snapshot = NULL;
+ else
+ snapshot = GetActiveSnapshot();
+
/* Can't use LargeObjectExists here because we need to specify snapshot */
if (!myLargeObjectExists(lobjId, snapshot))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("large object %u does not exist", lobjId)));
+ /* Apply permission checks, again specifying snapshot */
+ if ((descflags & IFS_RDLOCK) != 0)
+ {
+ if (!lo_compat_privileges &&
+ pg_largeobject_aclcheck_snapshot(lobjId,
+ GetUserId(),
+ ACL_SELECT,
+ snapshot) != ACLCHECK_OK)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for large object %u",
+ lobjId)));
+ }
+ if ((descflags & IFS_WRLOCK) != 0)
+ {
+ if (!lo_compat_privileges &&
+ pg_largeobject_aclcheck_snapshot(lobjId,
+ GetUserId(),
+ ACL_UPDATE,
+ snapshot) != ACLCHECK_OK)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for large object %u",
+ lobjId)));
+ }
+
+ /* OK to create a descriptor */
+ retval = (LargeObjectDesc *) MemoryContextAlloc(mcxt,
+ sizeof(LargeObjectDesc));
+ retval->id = lobjId;
+ retval->subid = GetCurrentSubTransactionId();
+ retval->offset = 0;
+ retval->flags = descflags;
+
/*
* We must register the snapshot in TopTransaction's resowner, because it
* must stay alive until the LO is closed rather than until the current
- * portal shuts down. Do this after checking that the LO exists, to avoid
- * leaking the snapshot if an error is thrown.
+ * portal shuts down. Do this last to avoid uselessly leaking the
+ * snapshot if an error is thrown above.
*/
if (snapshot)
snapshot = RegisterSnapshotOnOwner(snapshot,
TopTransactionResourceOwner);
-
- /* All set, create a descriptor */
- retval = (LargeObjectDesc *) MemoryContextAlloc(mcxt,
- sizeof(LargeObjectDesc));
- retval->id = lobjId;
- retval->subid = GetCurrentSubTransactionId();
- retval->offset = 0;
retval->snapshot = snapshot;
- retval->flags = descflags;
return retval;
}
@@ -330,7 +362,7 @@ inv_close(LargeObjectDesc *obj_desc)
/*
* Destroys an existing large object (not to be confused with a descriptor!)
*
- * returns -1 if failed
+ * Note we expect caller to have done any required permissions check.
*/
int
inv_drop(Oid lobjId)
@@ -351,6 +383,7 @@ inv_drop(Oid lobjId)
*/
CommandCounterIncrement();
+ /* For historical reasons, we always return 1 on success. */
return 1;
}
@@ -415,6 +448,11 @@ inv_seek(LargeObjectDesc *obj_desc, int64 offset, int whence)
Assert(PointerIsValid(obj_desc));
+ /*
+ * We allow seek/tell if you have either read or write permission, so no
+ * need for a permission check here.
+ */
+
/*
* Note: overflow in the additions is possible, but since we will reject
* negative results, we don't need any extra test for that.
@@ -457,6 +495,11 @@ inv_tell(LargeObjectDesc *obj_desc)
{
Assert(PointerIsValid(obj_desc));
+ /*
+ * We allow seek/tell if you have either read or write permission, so no
+ * need for a permission check here.
+ */
+
return obj_desc->offset;
}
@@ -476,6 +519,12 @@ inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes)
Assert(PointerIsValid(obj_desc));
Assert(buf != NULL);
+ if ((obj_desc->flags & IFS_RDLOCK) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for large object %u",
+ obj_desc->id)));
+
if (nbytes <= 0)
return 0;
@@ -581,7 +630,11 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
Assert(buf != NULL);
/* enforce writability because snapshot is probably wrong otherwise */
- Assert(obj_desc->flags & IFS_WRLOCK);
+ if ((obj_desc->flags & IFS_WRLOCK) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for large object %u",
+ obj_desc->id)));
if (nbytes <= 0)
return 0;
@@ -767,7 +820,11 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len)
Assert(PointerIsValid(obj_desc));
/* enforce writability because snapshot is probably wrong otherwise */
- Assert(obj_desc->flags & IFS_WRLOCK);
+ if ((obj_desc->flags & IFS_WRLOCK) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for large object %u",
+ obj_desc->id)));
/*
* use errmsg_internal here because we don't want to expose INT64_FORMAT
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 246fea8693..619b960e08 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -43,7 +43,6 @@
#include "commands/trigger.h"
#include "funcapi.h"
#include "libpq/auth.h"
-#include "libpq/be-fsstubs.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "miscadmin.h"
@@ -71,6 +70,7 @@
#include "storage/dsm_impl.h"
#include "storage/standby.h"
#include "storage/fd.h"
+#include "storage/large_object.h"
#include "storage/pg_shmem.h"
#include "storage/proc.h"
#include "storage/predicate.h"
diff --git a/src/include/libpq/be-fsstubs.h b/src/include/libpq/be-fsstubs.h
index 96bcaa0f08..e8107a2c9f 100644
--- a/src/include/libpq/be-fsstubs.h
+++ b/src/include/libpq/be-fsstubs.h
@@ -14,11 +14,6 @@
#ifndef BE_FSSTUBS_H
#define BE_FSSTUBS_H
-/*
- * compatibility option for access control
- */
-extern bool lo_compat_privileges;
-
/*
* These are not fmgr-callable, but are available to C code.
* Probably these should have had the underscore-free names,
diff --git a/src/include/storage/large_object.h b/src/include/storage/large_object.h
index 796a8fdeea..01d0985b44 100644
--- a/src/include/storage/large_object.h
+++ b/src/include/storage/large_object.h
@@ -27,9 +27,9 @@
* offset is the current seek offset within the LO
* flags contains some flag bits
*
- * NOTE: in current usage, flag bit IFS_RDLOCK is *always* set, and we don't
- * bother to test for it. Permission checks are made at first read or write
- * attempt, not during inv_open(), so we have other bits to remember that.
+ * NOTE: as of v11, permission checks are made when the large object is
+ * opened; therefore IFS_RDLOCK/IFS_WRLOCK indicate that read or write mode
+ * has been requested *and* the corresponding permission has been checked.
*
* NOTE: before 7.1, we also had to store references to the separate table
* and index of a specific large object. Now they all live in pg_largeobject
@@ -47,8 +47,6 @@ typedef struct LargeObjectDesc
/* bits in flags: */
#define IFS_RDLOCK (1 << 0) /* LO was opened for reading */
#define IFS_WRLOCK (1 << 1) /* LO was opened for writing */
-#define IFS_RD_PERM_OK (1 << 2) /* read permission has been verified */
-#define IFS_WR_PERM_OK (1 << 3) /* write permission has been verified */
} LargeObjectDesc;
@@ -78,6 +76,11 @@ typedef struct LargeObjectDesc
#define MAX_LARGE_OBJECT_SIZE ((int64) INT_MAX * LOBLKSIZE)
+/*
+ * GUC: backwards-compatibility flag to suppress LO permission checks
+ */
+extern bool lo_compat_privileges;
+
/*
* Function definitions...
*/
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 7f3b7ac25d..f07b610369 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1238,12 +1238,8 @@ SELECT lo_create(2002);
2002
(1 row)
-SELECT loread(lo_open(1001, x'20000'::int), 32); -- allowed, for now
- loread
---------
- \x
-(1 row)
-
+SELECT loread(lo_open(1001, x'20000'::int), 32); -- fail, wrong mode
+ERROR: large object descriptor 0 was not opened for reading
SELECT lowrite(lo_open(1001, x'40000'::int), 'abcd'); -- fail, wrong mode
ERROR: large object descriptor 0 was not opened for writing
SELECT loread(lo_open(1001, x'40000'::int), 32);
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index d22d08ca62..8978696fa6 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -779,7 +779,7 @@ SET SESSION AUTHORIZATION regress_user2;
SELECT lo_create(2001);
SELECT lo_create(2002);
-SELECT loread(lo_open(1001, x'20000'::int), 32); -- allowed, for now
+SELECT loread(lo_open(1001, x'20000'::int), 32); -- fail, wrong mode
SELECT lowrite(lo_open(1001, x'40000'::int), 'abcd'); -- fail, wrong mode
SELECT loread(lo_open(1001, x'40000'::int), 32);
--
2.14.1
From 080ce23a19a6f131ee11877f568be5ea861fdf2c Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 14 Aug 2017 11:15:31 +0900
Subject: [PATCH 2/3] Replace superuser checks of large object import/export by
ACL checks
The large object functions lo_import and lo_export are restricted to
superusers since their creation, and since Postgres 10 it is possible
to dump permission checks related to system functions. Let's remove
those hardcoded checks and replace them wiht proper GRANT permissions
at initialization. This also gives more flexibility to administrators.
---
src/backend/catalog/system_views.sql | 4 ++++
src/backend/libpq/be-fsstubs.c | 12 ------------
src/test/regress/expected/privileges.out | 10 ++++++----
src/test/regress/sql/privileges.sql | 2 ++
4 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index dc40cde424..cd7d0b208d 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1121,6 +1121,10 @@ AS 'jsonb_insert';
-- system to REVOKE access to those functions at initdb time. Administrators
-- can later change who can access these functions, or leave them as only
-- available to superuser / cluster owner, if they choose.
+REVOKE EXECUTE ON FUNCTION lo_import(text) FROM public;
+REVOKE EXECUTE ON FUNCTION lo_import(text, oid) FROM public;
+REVOKE EXECUTE ON FUNCTION lo_export(oid, text) FROM public;
+
REVOKE EXECUTE ON FUNCTION pg_start_backup(text, boolean, boolean) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stop_backup() FROM public;
REVOKE EXECUTE ON FUNCTION pg_stop_backup(boolean, boolean) FROM public;
diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 4d75c60979..507843c386 100644
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -448,12 +448,6 @@ lo_import_internal(text *filename, Oid lobjOid)
LargeObjectDesc *lobj;
Oid oid;
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to use server-side lo_import()"),
- errhint("Anyone can use the client-side lo_import() provided by libpq.")));
-
CreateFSContext();
/*
@@ -512,12 +506,6 @@ be_lo_export(PG_FUNCTION_ARGS)
LargeObjectDesc *lobj;
mode_t oumask;
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to use server-side lo_export()"),
- errhint("Anyone can use the client-side lo_export() provided by libpq.")));
-
CreateFSContext();
/*
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index f37df6c709..7f3b7ac25d 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1346,8 +1346,11 @@ ERROR: permission denied for large object 1002
SELECT lo_unlink(1002); -- to be denied
ERROR: must be owner of large object 1002
SELECT lo_export(1001, '/dev/null'); -- to be denied
-ERROR: must be superuser to use server-side lo_export()
-HINT: Anyone can use the client-side lo_export() provided by libpq.
+ERROR: permission denied for function lo_export
+SELECT lo_import('/dev/null'); -- to be denied
+ERROR: permission denied for function lo_import
+SELECT lo_import('/dev/null', 2003); -- to be denied
+ERROR: permission denied for function lo_import
\c -
SET lo_compat_privileges = true; -- compatibility mode
SET SESSION AUTHORIZATION regress_user4;
@@ -1376,8 +1379,7 @@ SELECT lo_unlink(1002);
(1 row)
SELECT lo_export(1001, '/dev/null'); -- to be denied
-ERROR: must be superuser to use server-side lo_export()
-HINT: Anyone can use the client-side lo_export() provided by libpq.
+ERROR: permission denied for function lo_export
-- don't allow unpriv users to access pg_largeobject contents
\c -
SELECT * FROM pg_largeobject LIMIT 0;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index e2c13e08a4..d22d08ca62 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -824,6 +824,8 @@ SELECT lo_truncate(lo_open(1002, x'20000'::int), 10); -- to be denied
SELECT lo_put(1002, 1, 'abcd'); -- to be denied
SELECT lo_unlink(1002); -- to be denied
SELECT lo_export(1001, '/dev/null'); -- to be denied
+SELECT lo_import('/dev/null'); -- to be denied
+SELECT lo_import('/dev/null', 2003); -- to be denied
\c -
SET lo_compat_privileges = true; -- compatibility mode
--
2.14.1
From d50917df2ddae1821329f1c4ee9a4965f734e474 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 14 Aug 2017 10:57:26 +0900
Subject: [PATCH 1/3] Remove ALLOW_DANGEROUS_LO_FUNCTIONS for LO-related
superuser checks
This switch dated of 4cd4a54c, which is old and not being used anymore by
modern distrubutions bundling PostgreSQL.
---
src/backend/libpq/be-fsstubs.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index bf45461b2f..4d75c60979 100644
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -448,13 +448,11 @@ lo_import_internal(text *filename, Oid lobjOid)
LargeObjectDesc *lobj;
Oid oid;
-#ifndef ALLOW_DANGEROUS_LO_FUNCTIONS
if (!superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to use server-side lo_import()"),
errhint("Anyone can use the client-side lo_import() provided by libpq.")));
-#endif
CreateFSContext();
@@ -514,13 +512,11 @@ be_lo_export(PG_FUNCTION_ARGS)
LargeObjectDesc *lobj;
mode_t oumask;
-#ifndef ALLOW_DANGEROUS_LO_FUNCTIONS
if (!superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to use server-side lo_export()"),
errhint("Anyone can use the client-side lo_export() provided by libpq.")));
-#endif
CreateFSContext();
--
2.14.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
