8000 Replace last PushOverrideSearchPath() call with set_config_option(). · postgres/postgres@23cb8ea · GitHub
[go: up one dir, main page]
Skip to content

Commit 23cb8ea

Browse files
nmischnmisch
committed
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
1 parent 625acd0 commit 23cb8ea

File tree

4 files changed

+100
-10
lines changed

4 files changed

+100
-10
lines changed

‎src/backend/catalog/namespace.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3455,6 +3455,10 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
34553455
/*
34563456
* PushOverrideSearchPath - temporarily override the search path
34573457
*
3458+
* Do not use this function; almost any usage introduces a security
3459+
* vulnerability. It exists for the benefit of legacy code running in
3460+
* non-security-sensitive environments.
3461+
*
34583462
* We allow nested overrides, hence the push/pop terminology. The GUC
34593463
* search_path variable is ignored while an override is active.
34603464
*

‎src/backend/commands/schemacmds.c

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "commands/schemacmds.h"
3030
#include "miscadmin.h"
3131
#include "parser/parse_utilcmd.h"
32+
#include "parser/scansup.h"
3233
#include "tcop/utility.h"
3334
#include "utils/acl.h"
3435
#include "utils/builtins.h"
@@ -53,14 +54,16 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
5354
{
5455
const char *schemaName = stmt->schemaname;
5556
Oid namespaceId;
56-
OverrideSearchPath *overridePath;
5757
List *parsetree_list;
5858
ListCell *parsetree_item;
5959
Oid owner_uid;
6060
Oid saved_uid;
6161
int save_sec_context;
62+
int save_nestlevel;
63+
char *nsp = namespace_search_path;
6264
AclResult aclresult;
6365
ObjectAddress address;
66+
StringInfoData pathbuf;
6467

6568
GetUserIdAndSecContext(&saved_uid, &save_sec_context);
6669

@@ -153,14 +156,26 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
153156
CommandCounterIncrement();
154157

155158
/*
156-
* Temporarily make the new namespace be the front of the search path, as
157-
* well as the default creation target namespace. This will be undone at
158-
* the end of this routine, or upon error.
159+
* Prepend the new schema to the current search path.
160+
*
161+
* We use the equivalent of a function SET option to allow the setting to
162+
* persist for exactly the duration of the schema creation. guc.c also
163+
* takes care of undoing the setting on error.
159164
*/
160-
overridePath = GetOverrideSearchPath(CurrentMemoryContext);
161-
overridePath->schemas = lcons_oid(namespaceId, overridePath->schemas);
162-
/* XXX should we clear overridePath->useTemp? */
163-
PushOverrideSearchPath(overridePath);
165+
save_nestlevel = NewGUCNestLevel();
166+
167+
initStringInfo(&pathbuf);
168+
appendStringInfoString(&pathbuf, quote_identifier(schemaName));
169+
170+
while (scanner_isspace(*nsp))
171+
nsp++;
172+
173+
if (*nsp != '\0')
174+
appendStringInfo(&pathbuf, ", %s", nsp);
175+
176+
(void) set_config_option("search_path", pathbuf.data,
177+
PGC_USERSET, PGC_S_SESSION,
178+
GUC_ACTION_SAVE, true, 0, false);
164179

165180
/*
166181
* Report the new schema to possibly interested event triggers. Note we
@@ -214,8 +229,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
214229
CommandCounterIncrement();
215230
}
216231

217-
/* Reset search path to normal state */
218-
PopOverrideSearchPath();
232+
/*
233+
* Restore the GUC variable search_path we set above.
234+
*/
235+
AtEOXact_GUC(true, save_nestlevel);
219236

220237
/* Reset current user and security context */
221238
SetUserIdAndSecContext(saved_uid, save_sec_context);

‎src/test/regress/expected/namespace.out

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
--
22
-- Regression tests for schemas (namespaces)
33
--
4+
-- set the whitespace-only search_path to test that the
5+
-- GUC list syntax is preserved during a schema creation
6+
SELECT pg_catalog.set_config('search_path', ' ', false);
7+
set_config
8+
------------
9+
10+
(1 row)
11+
412
CREATE SCHEMA test_ns_schema_1
513
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
614
CREATE VIEW abc_view AS
@@ -9,6 +17,43 @@ CREATE SCHEMA test_ns_schema_1
917
a serial,
1018
b int UNIQUE
1119
);
20+
-- verify that the correct search_path restored on abort
21+
SET search_path to public;
22+
BEGIN;
23+
SET search_path to public, test_ns_schema_1;
24+
CREATE SCHEMA test_ns_schema_2
25+
CREATE VIEW abc_view AS SELECT c FROM abc;
26+
ERROR: column "c" does not exist
27+
LINE 2: CREATE VIEW abc_view AS SELECT c FROM abc;
28+
^
29+
COMMIT;
30+
SHOW search_path;
31+
search_path
32+
-------------
33+
public
34+
(1 row)
35+
36+
-- verify that the correct search_path preserved
37+
-- after creating the schema and on commit
38+
BEGIN;
39+
SET search_path to public, test_ns_schema_1;
40+
CREATE SCHEMA test_ns_schema_2
41+
CREATE VIEW abc_view AS SELECT a FROM abc;
42+
SHOW search_path;
43+
search_path
44+
--------------------------
45+
public, test_ns_schema_1
46+
(1 row)
47+
48+
COMMIT;
49+
SHOW search_path;
50+
search_path
51+
--------------------------
52+
public, test_ns_schema_1
53+
(1 row)
54+
55+
DROP SCHEMA test_ns_schema_2 CASCADE;
56+
NOTICE: drop cascades to view test_ns_schema_2.abc_view
1257
-- verify that the objects were created
1358
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
1459
(SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1');

‎src/test/regress/sql/namespace.sql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
-- Regression tests for schemas (namespaces)
33
--
44

5+
-- set the whitespace-only search_path to test that the
6+
-- GUC list syntax is preserved during a schema creation
7+
SELECT pg_catalog.set_config('search_path', ' ', false);
8+
59
CREATE SCHEMA test_ns_schema_1
610
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
711

@@ -13,6 +17,26 @@ CREATE SCHEMA test_ns_schema_1
1317
b int UNIQUE
1418
);
1519

20+
-- verify that the correct search_path restored on abort
21+
SET search_path to public;
22+
BEGIN;
23+
SET search_path to public, test_ns_schema_1;
24+
CREATE SCHEMA test_ns_schema_2
25+
CREATE VIEW abc_view AS SELECT c FROM abc;
26+
COMMIT;
27+
SHOW search_path;
28+
29+
-- verify that the correct search_path preserved
30+
-- after creating the schema and on commit
31+
BEGIN;
32+
SET search_path to public, test_ns_schema_1;
33+
CREATE SCHEMA test_ns_schema_2
34+
CREATE VIEW abc_view AS SELECT a FROM abc;
35+
SHOW search_path;
36+
COMMIT;
37+
SHOW search_path;
38+
DROP SCHEMA test_ns_schema_2 CASCADE;
39+
1640
-- verify that the objects were created
1741
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
1842
(SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1');

0 commit comments

Comments
 (0)
0