Skip to content

Commit 681d9e4

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 b8c3f6d commit 681d9e4

File tree

7 files changed

+165
-11
lines changed

7 files changed

+165
-11
lines changed

‎contrib/seg/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ PGFILEDESC = "seg - line segment data type"
1414

1515
HEADERS = segdata.h
1616

17-
REGRESS = seg
17+
REGRESS = security seg
1818

1919
EXTRA_CLEAN = y.tab.c y.tab.h
2020

‎contrib/seg/expected/security.out

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--
2+
-- Test extension script protection against search path overriding
3+
--
4+
CREATE ROLE regress_seg_role;
5+
SELECT current_database() AS datname \gset
6+
GRANT CREATE ON DATABASE :"datname" TO regress_seg_role;
7+
SET ROLE regress_seg_role;
8+
CREATE SCHEMA regress_seg_schema;
9+
CREATE FUNCTION regress_seg_schema.exfun(i int) RETURNS int AS $$
10+
BEGIN
11+
CREATE EXTENSION seg VERSION '1.2';
12+
13+
CREATE FUNCTION regress_seg_schema.compare(oid, regclass) RETURNS boolean AS
14+
'BEGIN RAISE EXCEPTION ''overloaded compare() called by %'', current_user; END;' LANGUAGE plpgsql;
15+
16+
CREATE OPERATOR = (LEFTARG = oid, RIGHTARG = regclass, PROCEDURE = regress_seg_schema.compare);
17+
18+
ALTER EXTENSION seg UPDATE TO '1.3';
19+
20+
RETURN i;
21+
END; $$ LANGUAGE plpgsql;
22+
CREATE SCHEMA test_schema
23+
CREATE TABLE t(i int) PARTITION BY RANGE (i)
24+
CREATE TABLE p1 PARTITION OF t FOR VALUES FROM (1) TO (regress_seg_schema.exfun(2));
25+
DROP SCHEMA test_schema CASCADE;
26+
NOTICE: drop cascades to 3 other objects
27+
DETAIL: drop cascades to table test_schema.t
28+
drop cascades to extension seg
29+
drop cascades to operator test_schema.=(oid,regclass)
30+
RESET ROLE;
31+
DROP OWNED BY regress_seg_role;
32+
DROP ROLE regress_seg_role;

‎contrib/seg/sql/security.sql

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--
2+
-- Test extension script protection against search path overriding
3+
--
4+
5+
CREATE ROLE regress_seg_role;
6+
SELECT current_database() AS datname \gset
7+
GRANT CREATE ON DATABASE :"datname" TO regress_seg_role;
8+
SET ROLE regress_seg_role;
9+
CREATE SCHEMA regress_seg_schema;
10+
11+
CREATE FUNCTION regress_seg_schema.exfun(i int) RETURNS int AS $$
12+
BEGIN
13+
CREATE EXTENSION seg VERSION '1.2';
14+
15+
CREATE FUNCTION regress_seg_schema.compare(oid, regclass) RETURNS boolean AS
16+
'BEGIN RAISE EXCEPTION ''overloaded compare() called by %'', current_user; END;' LANGUAGE plpgsql;
17+
18+
CREATE OPERATOR = (LEFTARG = oid, RIGHTARG = regclass, PROCEDURE = regress_seg_schema.compare);
19+
20+
ALTER EXTENSION seg UPDATE TO '1.3';
21+
22+
RETURN i;
23+
END; $$ LANGUAGE plpgsql;
24+
25+
CREATE SCHEMA test_schema
26+
CREATE TABLE t(i int) PARTITION BY RANGE (i)
27+
CREATE TABLE p1 PARTITION OF t FOR VALUES FROM (1) TO (regress_seg_schema.exfun(2));
28+
29+
DROP SCHEMA test_schema CASCADE;
30+
RESET ROLE;
31+
DROP OWNED BY regress_seg_role;
32+
DROP ROLE regress_seg_role;

‎src/backend/catalog/namespace.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3515,6 +3515,10 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
35153515
/*
35163516
* PushOverrideSearchPath - temporarily override the search path
35173517
*
3518+
* Do not use this function; almost any usage introduces a security
3519+
* vulnerability. It exists for the benefit of legacy code running in
3520+
* non-security-sensitive environments.
3521+
*
35183522
* We allow nested overrides, hence the push/pop terminology. The GUC
35193523
* search_path variable is ignored while an override is active.
35203524
*

‎src/backend/commands/schemacmds.c

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "commands/schemacmds.h"
3131
#include "miscadmin.h"
3232
#include "parser/parse_utilcmd.h"
33+
#include "parser/scansup.h"
3334
#include "tcop/utility.h"
3435
#include "utils/acl.h"
3536
#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
@@ -215,8 +230,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
215230
CommandCounterIncrement();
216231
}
217232

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

221238
/* Reset current user and security context */
222239
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)