summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNoah Misch <noah@leadboat.com>2023-05-08 06:14:07 -0700
committerNoah Misch <noah@leadboat.com>2023-05-08 06:14:07 -0700
commit681d9e4621aac0a9c71364b6f54f00f6d8c4337f (patch)
tree058735d8b659a1a69e6e296be14d59d45ff70b21
parentb8c3f6df85e78e09c9709fc895aced719aebd7f9 (diff)
downloadpostgresql-681d9e4621aac0a9c71364b6f54f00f6d8c4337f.tar.gz
Replace last PushOverrideSearchPath() call with set_config_option().
The two methods don't cooperate, so set_config_option("search_path", ...) has been ineffective under non-empty overrideStack. This defect enabled an attacker having database-level CREATE privilege to execute arbitrary code as the bootstrap superuser. While that particular attack requires v13+ for the trusted extension attribute, other attacks are feasible in all supported versions. Standardize on the combination of NewGUCNestLevel() and set_config_option("search_path", ...). It is newer than PushOverrideSearchPath(), more-prevalent, and has no known disadvantages. The "override" mechanism remains for now, for compatibility with out-of-tree code. Users should update such code, which likely suffers from the same sort of vulnerability closed here. Back-patch to v11 (all supported versions). Alexander Lakhin. Reported by Alexander Lakhin. Security: CVE-2023-2454
-rw-r--r--contrib/seg/Makefile2
-rw-r--r--contrib/seg/expected/security.out32
-rw-r--r--contrib/seg/sql/security.sql32
-rw-r--r--src/backend/catalog/namespace.c4
-rw-r--r--src/backend/commands/schemacmds.c37
-rw-r--r--src/test/regress/expected/namespace.out45
-rw-r--r--src/test/regress/sql/namespace.sql24
7 files changed, 165 insertions, 11 deletions
diff --git a/contrib/seg/Makefile b/contrib/seg/Makefile
index c6c134b8f1..a1e49bf051 100644
--- a/contrib/seg/Makefile
+++ b/contrib/seg/Makefile
@@ -14,7 +14,7 @@ PGFILEDESC = "seg - line segment data type"
HEADERS = segdata.h
-REGRESS = seg
+REGRESS = security seg
EXTRA_CLEAN = y.tab.c y.tab.h
diff --git a/contrib/seg/expected/security.out b/contrib/seg/expected/security.out
new file mode 100644
index 0000000000..b47598d039
--- /dev/null
+++ b/contrib/seg/expected/security.out
@@ -0,0 +1,32 @@
+--
+-- Test extension script protection against search path overriding
+--
+CREATE ROLE regress_seg_role;
+SELECT current_database() AS datname \gset
+GRANT CREATE ON DATABASE :"datname" TO regress_seg_role;
+SET ROLE regress_seg_role;
+CREATE SCHEMA regress_seg_schema;
+CREATE FUNCTION regress_seg_schema.exfun(i int) RETURNS int AS $$
+BEGIN
+ CREATE EXTENSION seg VERSION '1.2';
+
+ CREATE FUNCTION regress_seg_schema.compare(oid, regclass) RETURNS boolean AS
+ 'BEGIN RAISE EXCEPTION ''overloaded compare() called by %'', current_user; END;' LANGUAGE plpgsql;
+
+ CREATE OPERATOR = (LEFTARG = oid, RIGHTARG = regclass, PROCEDURE = regress_seg_schema.compare);
+
+ ALTER EXTENSION seg UPDATE TO '1.3';
+
+ RETURN i;
+END; $$ LANGUAGE plpgsql;
+CREATE SCHEMA test_schema
+CREATE TABLE t(i int) PARTITION BY RANGE (i)
+CREATE TABLE p1 PARTITION OF t FOR VALUES FROM (1) TO (regress_seg_schema.exfun(2));
+DROP SCHEMA test_schema CASCADE;
+NOTICE: drop cascades to 3 other objects
+DETAIL: drop cascades to table test_schema.t
+drop cascades to extension seg
+drop cascades to operator test_schema.=(oid,regclass)
+RESET ROLE;
+DROP OWNED BY regress_seg_role;
+DROP ROLE regress_seg_role;
diff --git a/contrib/seg/sql/security.sql b/contrib/seg/sql/security.sql
new file mode 100644
index 0000000000..7dfbbaa304
--- /dev/null
+++ b/contrib/seg/sql/security.sql
@@ -0,0 +1,32 @@
+--
+-- Test extension script protection against search path overriding
+--
+
+CREATE ROLE regress_seg_role;
+SELECT current_database() AS datname \gset
+GRANT CREATE ON DATABASE :"datname" TO regress_seg_role;
+SET ROLE regress_seg_role;
+CREATE SCHEMA regress_seg_schema;
+
+CREATE FUNCTION regress_seg_schema.exfun(i int) RETURNS int AS $$
+BEGIN
+ CREATE EXTENSION seg VERSION '1.2';
+
+ CREATE FUNCTION regress_seg_schema.compare(oid, regclass) RETURNS boolean AS
+ 'BEGIN RAISE EXCEPTION ''overloaded compare() called by %'', current_user; END;' LANGUAGE plpgsql;
+
+ CREATE OPERATOR = (LEFTARG = oid, RIGHTARG = regclass, PROCEDURE = regress_seg_schema.compare);
+
+ ALTER EXTENSION seg UPDATE TO '1.3';
+
+ RETURN i;
+END; $$ LANGUAGE plpgsql;
+
+CREATE SCHEMA test_schema
+CREATE TABLE t(i int) PARTITION BY RANGE (i)
+CREATE TABLE p1 PARTITION OF t FOR VALUES FROM (1) TO (regress_seg_schema.exfun(2));
+
+DROP SCHEMA test_schema CASCADE;
+RESET ROLE;
+DROP OWNED BY regress_seg_role;
+DROP ROLE regress_seg_role;
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 14e57adee2..73ddb67882 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3515,6 +3515,10 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
/*
* PushOverrideSearchPath - temporarily override the search path
*
+ * Do not use this function; almost any usage introduces a security
+ * vulnerability. It exists for the benefit of legacy code running in
+ * non-security-sensitive environments.
+ *
* We allow nested overrides, hence the push/pop terminology. The GUC
* search_path variable is ignored while an override is active.
*
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 48590247f8..b6a71154a8 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -30,6 +30,7 @@
#include "commands/schemacmds.h"
#include "miscadmin.h"
#include "parser/parse_utilcmd.h"
+#include "parser/scansup.h"
#include "tcop/utility.h"
#include "utils/acl.h"
#include "utils/builtins.h"
@@ -53,14 +54,16 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
{
const char *schemaName = stmt->schemaname;
Oid namespaceId;
- OverrideSearchPath *overridePath;
List *parsetree_list;
ListCell *parsetree_item;
Oid owner_uid;
Oid saved_uid;
int save_sec_context;
+ int save_nestlevel;
+ char *nsp = namespace_search_path;
AclResult aclresult;
ObjectAddress address;
+ StringInfoData pathbuf;
GetUserIdAndSecContext(&saved_uid, &save_sec_context);
@@ -153,14 +156,26 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
CommandCounterIncrement();
/*
- * Temporarily make the new namespace be the front of the search path, as
- * well as the default creation target namespace. This will be undone at
- * the end of this routine, or upon error.
+ * Prepend the new schema to the current search path.
+ *
+ * We use the equivalent of a function SET option to allow the setting to
+ * persist for exactly the duration of the schema creation. guc.c also
+ * takes care of undoing the setting on error.
*/
- overridePath = GetOverrideSearchPath(CurrentMemoryContext);
- overridePath->schemas = lcons_oid(namespaceId, overridePath->schemas);
- /* XXX should we clear overridePath->useTemp? */
- PushOverrideSearchPath(overridePath);
+ save_nestlevel = NewGUCNestLevel();
+
+ initStringInfo(&pathbuf);
+ appendStringInfoString(&pathbuf, quote_identifier(schemaName));
+
+ while (scanner_isspace(*nsp))
+ nsp++;
+
+ if (*nsp != '\0')
+ appendStringInfo(&pathbuf, ", %s", nsp);
+
+ (void) set_config_option("search_path", pathbuf.data,
+ PGC_USERSET, PGC_S_SESSION,
+ GUC_ACTION_SAVE, true, 0, false);
/*
* Report the new schema to possibly interested event triggers. Note we
@@ -215,8 +230,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
CommandCounterIncrement();
}
- /* Reset search path to normal state */
- PopOverrideSearchPath();
+ /*
+ * Restore the GUC variable search_path we set above.
+ */
+ AtEOXact_GUC(true, save_nestlevel);
/* Reset current user and security context */
SetUserIdAndSecContext(saved_uid, save_sec_context);
diff --git a/src/test/regress/expected/namespace.out b/src/test/regress/expected/namespace.out
index 2564d1b080..a62fd8ded0 100644
--- a/src/test/regress/expected/namespace.out
+++ b/src/test/regress/expected/namespace.out
@@ -1,6 +1,14 @@
--
-- Regression tests for schemas (namespaces)
--
+-- set the whitespace-only search_path to test that the
+-- GUC list syntax is preserved during a schema creation
+SELECT pg_catalog.set_config('search_path', ' ', false);
+ set_config
+------------
+
+(1 row)
+
CREATE SCHEMA test_ns_schema_1
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
CREATE VIEW abc_view AS
@@ -9,6 +17,43 @@ CREATE SCHEMA test_ns_schema_1
a serial,
b int UNIQUE
);
+-- verify that the correct search_path restored on abort
+SET search_path to public;
+BEGIN;
+SET search_path to public, test_ns_schema_1;
+CREATE SCHEMA test_ns_schema_2
+ CREATE VIEW abc_view AS SELECT c FROM abc;
+ERROR: column "c" does not exist
+LINE 2: CREATE VIEW abc_view AS SELECT c FROM abc;
+ ^
+COMMIT;
+SHOW search_path;
+ search_path
+-------------
+ public
+(1 row)
+
+-- verify that the correct search_path preserved
+-- after creating the schema and on commit
+BEGIN;
+SET search_path to public, test_ns_schema_1;
+CREATE SCHEMA test_ns_schema_2
+ CREATE VIEW abc_view AS SELECT a FROM abc;
+SHOW search_path;
+ search_path
+--------------------------
+ public, test_ns_schema_1
+(1 row)
+
+COMMIT;
+SHOW search_path;
+ search_path
+--------------------------
+ public, test_ns_schema_1
+(1 row)
+
+DROP SCHEMA test_ns_schema_2 CASCADE;
+NOTICE: drop cascades to view test_ns_schema_2.abc_view
-- verify that the objects were created
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
(SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1');
diff --git a/src/test/regress/sql/namespace.sql b/src/test/regress/sql/namespace.sql
index 6b12c96193..3474f5ecf4 100644
--- a/src/test/regress/sql/namespace.sql
+++ b/src/test/regress/sql/namespace.sql
@@ -2,6 +2,10 @@
-- Regression tests for schemas (namespaces)
--
+-- set the whitespace-only search_path to test that the
+-- GUC list syntax is preserved during a schema creation
+SELECT pg_catalog.set_config('search_path', ' ', false);
+
CREATE SCHEMA test_ns_schema_1
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
@@ -13,6 +17,26 @@ CREATE SCHEMA test_ns_schema_1
b int UNIQUE
);
+-- verify that the correct search_path restored on abort
+SET search_path to public;
+BEGIN;
+SET search_path to public, test_ns_schema_1;
+CREATE SCHEMA test_ns_schema_2
+ CREATE VIEW abc_view AS SELECT c FROM abc;
+COMMIT;
+SHOW search_path;
+
+-- verify that the correct search_path preserved
+-- after creating the schema and on commit
+BEGIN;
+SET search_path to public, test_ns_schema_1;
+CREATE SCHEMA test_ns_schema_2
+ CREATE VIEW abc_view AS SELECT a FROM abc;
+SHOW search_path;
+COMMIT;
+SHOW search_path;
+DROP SCHEMA test_ns_schema_2 CASCADE;
+
-- verify that the objects were created
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
(SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1');