From 5ecc0d738e5864848bbc2d1d97e56d5846624ba2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 9 Nov 2017 12:36:58 -0500 Subject: [PATCH] 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 --- src/backend/catalog/system_views.sql | 10 ++++++++++ src/backend/libpq/be-fsstubs.c | 16 ---------------- src/include/catalog/catversion.h | 2 +- src/include/pg_config_manual.h | 10 ---------- src/test/regress/expected/privileges.out | 10 ++++++---- src/test/regress/sql/privileges.sql | 2 ++ 6 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index dc40cde424..394aea8e0f 100644 --- 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 84c2d26402..50c70dd66d 100644 --- 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 9a7f5b25a3..39c70b415a 100644 --- 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 b048175321..6f2238b330 100644 --- 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 65d950f15b..771971a095 100644 --- 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 902f64c747..a900ba2f84 100644 --- 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 -- 2.40.0