Skip to content

Commit 01e8182

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 76a3e1d commit 01e8182

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
@@ -13,7 +13,7 @@ PGFILEDESC = "seg - line segment data type"
1313

1414
HEADERS = segdata.h
1515

16-
REGRESS = seg
16+
REGRESS = security seg
1717

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

‎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
@@ -3518,6 +3518,10 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
35183518
/*
35193519
* PushOverrideSearchPath - temporarily override the search path
35203520
*
3521+
* Do not use this function; almost any usage introduces a security
3522+
* vulnerability. It exists for the benefit of legacy code running in
3523+
* non-security-sensitive environments.
3524+
*
35213525
* We allow nested overrides, hence the push/pop terminology. The GUC
35223526
* search_path variable is ignored while an override is active.
35233527
*

‎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"
@@ -52,14 +53,16 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
5253
{
5354
const char *schemaName = stmt->schemaname;
5455
Oid namespaceId;
55-
OverrideSearchPath *overridePath;
5656
List *parsetree_list;
5757
ListCell *parsetree_item;
5858
Oid owner_uid;
5959
Oid saved_uid;
6060
int save_sec_context;
61+
int save_nestlevel;
62+
char *nsp = namespace_search_path;
6163
AclResult aclresult;
6264
ObjectAddress address;
65+
StringInfoData pathbuf;
6366

6467
GetUserIdAndSecContext(&saved_uid, &save_sec_context);
6568

@@ -152,14 +155,26 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
152155
CommandCounterIncrement();
153156

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

164179
/*
165180
* 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)