You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
250 lines
9.0 KiB
250 lines
9.0 KiB
From 681d9e4621aac0a9c71364b6f54f00f6d8c4337f Mon Sep 17 00:00:00 2001
|
|
From 8d525d7b9545884a3e0d79adcd61543f9ae2ae28 Mon Sep 17 00:00:00 2001
|
|
From: Noah Misch <noah@leadboat.com>
|
|
Date: Mon, 8 May 2023 06:14:07 -0700
|
|
Subject: 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
|
|
---
|
|
contrib/seg/Makefile | 2 +-
|
|
contrib/seg/expected/security.out | 32 ++++++++++++++++++
|
|
contrib/seg/sql/security.sql | 32 ++++++++++++++++++
|
|
src/backend/catalog/namespace.c | 4 +++
|
|
src/backend/commands/schemacmds.c | 37 ++++++++++++++------
|
|
src/test/regress/expected/namespace.out | 45 +++++++++++++++++++++++++
|
|
src/test/regress/sql/namespace.sql | 24 +++++++++++++
|
|
7 files changed, 165 insertions(+), 11 deletions(-)
|
|
create mode 100644 contrib/seg/expected/security.out
|
|
create mode 100644 contrib/seg/sql/security.sql
|
|
|
|
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_schema_1
|
|
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
|
|
CREATE VIEW abc_view AS
|
|
@@ -9,6 +17,43 @@ CREATE SCHEMA test_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_schema_1;
|
|
+CREATE SCHEMA test_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_schema_1;
|
|
+CREATE SCHEMA test_schema_2
|
|
+ CREATE VIEW abc_view AS SELECT a FROM abc;
|
|
+SHOW search_path;
|
|
+ search_path
|
|
+-----------------------
|
|
+ public, test_schema_1
|
|
+(1 row)
|
|
+
|
|
+COMMIT;
|
|
+SHOW search_path;
|
|
+ search_path
|
|
+-----------------------
|
|
+ public, test_schema_1
|
|
+(1 row)
|
|
+
|
|
+DROP SCHEMA test_schema_2 CASCADE;
|
|
+NOTICE: drop cascades to view test_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_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_schema_1
|
|
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
|
|
|
|
@@ -13,6 +17,26 @@ CREATE SCHEMA test_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_schema_1;
|
|
+CREATE SCHEMA test_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_schema_1;
|
|
+CREATE SCHEMA test_schema_2
|
|
+ CREATE VIEW abc_view AS SELECT a FROM abc;
|
|
+SHOW search_path;
|
|
+COMMIT;
|
|
+SHOW search_path;
|
|
+DROP SCHEMA test_schema_2 CASCADE;
|
|
+
|
|
-- verify that the objects were created
|
|
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
|
|
(SELECT oid FROM pg_namespace WHERE nspname = 'test_schema_1');
|
|
diff --git a/contrib/sepgsql/expected/ddl.out b/contrib/sepgsql/expected/ddl.out
|
|
index e8da587564..15d2b9c5e7 100644
|
|
--- a/contrib/sepgsql/expected/ddl.out
|
|
+++ b/contrib/sepgsql/expected/ddl.out
|
|
@@ -24,7 +24,6 @@ LOG: SELinux: allowed { create } scontext=unconfined_u:unconfined_r:sepgsql_reg
|
|
CREATE USER regress_sepgsql_test_user;
|
|
CREATE SCHEMA regtest_schema;
|
|
LOG: SELinux: allowed { create } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema"
|
|
-LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=system_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="public"
|
|
GRANT ALL ON SCHEMA regtest_schema TO regress_sepgsql_test_user;
|
|
SET search_path = regtest_schema, public;
|
|
CREATE TABLE regtest_table (x serial primary key, y text);
|
|
--
|
|
2.41.0
|
|
|