Restrict lo_import()/lo_export() via SQL permissions not hard-wired checks.

While it's generally unwise to give permissions on these functions to
anyone but a superuser, we've been moving away from hard-wired permission
checks inside functions in favor of using the SQL permission system to
control access.  Bring lo_import() and lo_export() into compliance with
that approach.

In particular, this removes the manual configuration option
ALLOW_DANGEROUS_LO_FUNCTIONS.  That dates back to 1999 (commit 4cd4a54c8);
it's unlikely anyone has used it in many years.  Moreover, if you really
want such behavior, now you can get it with GRANT ... TO PUBLIC instead.

Michael Paquier

Discussion: https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index dc40cde4240ed862fd7c45b2111a60621e82ae6d..394aea8e0f5ff2a226a8fe373059b7827150c79c 100644 (file)
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1115,12 +1115,14 @@ LANGUAGE INTERNAL
 STRICT IMMUTABLE PARALLEL SAFE
 AS 'jsonb_insert';
 
+--
 -- The default permissions for functions mean that anyone can execute them.
 -- A number of functions shouldn't be executable by just anyone, but rather
 -- than use explicit 'superuser()' checks in those functions, we use the GRANT
 -- 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 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;
@@ -1138,8 +1140,16 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public;
 
+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_ls_logdir() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+
+--
+-- We also set up some things as accessible to standard roles.
+--
 GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
 GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
 

diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 84c2d26402cc5b6a6f8b4bd0a3fbd316394011eb..50c70dd66d6975feab4fb72defd3d7650761a8d6 100644 (file)
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -448,14 +448,6 @@ 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,14 +506,6 @@ 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();
 
        /*

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9a7f5b25a3a17d3647178b9ce277985a93fda611..39c70b415aed51d148a7f7c2e63a664306957d64 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     201710161
+#define CATALOG_VERSION_NO     201711091
 
 #endif

diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index b0481753210a9987dccbed891411d09aa5d02a7e..6f2238b330a277f27538f60020e2ecfe14cd63a6 100644 (file)
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -72,16 +72,6 @@
  */
 #define NUM_ATOMICS_SEMAPHORES         64
 
-/*
- * Define this if you want to allow the lo_import and lo_export SQL
- * functions to be executed by ordinary users.  By default these
- * functions are only available to the Postgres superuser.  CAUTION:
- * These functions are SECURITY HOLES since they can read and write
- * any file that the PostgreSQL server has permission to access.  If
- * you turn this on, don't say we didn't warn you.
- */
-/* #define ALLOW_DANGEROUS_LO_FUNCTIONS */
-
 /*
  * MAXPGPATH: standard size of a pathname buffer in PostgreSQL (hence,
  * maximum usable pathname length is one less).

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 65d950f15b1d45496b619610aedd793fdb985265..771971a095fd831d41ffbefa365b8c672a8f6c20 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1358,8 +1358,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;
@@ -1388,8 +1391,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 902f64c747c355101ece0c63989dd6d36ed4b613..a900ba2f8495f00f5660344ec70a87e0315d7d73 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -839,6 +839,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