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
This commit is contained in:
Noah Misch 2023-05-08 06:14:07 -07:00
parent b8c3f6df85
commit 681d9e4621
7 changed files with 165 additions and 11 deletions

View File

@ -14,7 +14,7 @@ PGFILEDESC = "seg - line segment data type"
HEADERS = segdata.h HEADERS = segdata.h
REGRESS = seg REGRESS = security seg
EXTRA_CLEAN = y.tab.c y.tab.h EXTRA_CLEAN = y.tab.c y.tab.h

View File

@ -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;

View File

@ -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;

View File

@ -3515,6 +3515,10 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
/* /*
* PushOverrideSearchPath - temporarily override the search 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 * We allow nested overrides, hence the push/pop terminology. The GUC
* search_path variable is ignored while an override is active. * search_path variable is ignored while an override is active.
* *

View File

@ -30,6 +30,7 @@
#include "commands/schemacmds.h" #include "commands/schemacmds.h"
#include "miscadmin.h" #include "miscadmin.h"
#include "parser/parse_utilcmd.h" #include "parser/parse_utilcmd.h"
#include "parser/scansup.h"
#include "tcop/utility.h" #include "tcop/utility.h"
#include "utils/acl.h" #include "utils/acl.h"
#include "utils/builtins.h" #include "utils/builtins.h"
@ -53,14 +54,16 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
{ {
const char *schemaName = stmt->schemaname; const char *schemaName = stmt->schemaname;
Oid namespaceId; Oid namespaceId;
OverrideSearchPath *overridePath;
List *parsetree_list; List *parsetree_list;
ListCell *parsetree_item; ListCell *parsetree_item;
Oid owner_uid; Oid owner_uid;
Oid saved_uid; Oid saved_uid;
int save_sec_context; int save_sec_context;
int save_nestlevel;
char *nsp = namespace_search_path;
AclResult aclresult; AclResult aclresult;
ObjectAddress address; ObjectAddress address;
StringInfoData pathbuf;
GetUserIdAndSecContext(&saved_uid, &save_sec_context); GetUserIdAndSecContext(&saved_uid, &save_sec_context);
@ -153,14 +156,26 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
CommandCounterIncrement(); CommandCounterIncrement();
/* /*
* Temporarily make the new namespace be the front of the search path, as * Prepend the new schema to the current search path.
* well as the default creation target namespace. This will be undone at *
* the end of this routine, or upon error. * 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); save_nestlevel = NewGUCNestLevel();
overridePath->schemas = lcons_oid(namespaceId, overridePath->schemas);
/* XXX should we clear overridePath->useTemp? */ initStringInfo(&pathbuf);
PushOverrideSearchPath(overridePath); 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 * Report the new schema to possibly interested event triggers. Note we
@ -215,8 +230,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
CommandCounterIncrement(); 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 */ /* Reset current user and security context */
SetUserIdAndSecContext(saved_uid, save_sec_context); SetUserIdAndSecContext(saved_uid, save_sec_context);

View File

@ -1,6 +1,14 @@
-- --
-- Regression tests for schemas (namespaces) -- 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 SCHEMA test_ns_schema_1
CREATE UNIQUE INDEX abc_a_idx ON abc (a) CREATE UNIQUE INDEX abc_a_idx ON abc (a)
CREATE VIEW abc_view AS CREATE VIEW abc_view AS
@ -9,6 +17,43 @@ CREATE SCHEMA test_ns_schema_1
a serial, a serial,
b int UNIQUE 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 -- verify that the objects were created
SELECT COUNT(*) FROM pg_class WHERE relnamespace = SELECT COUNT(*) FROM pg_class WHERE relnamespace =
(SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1'); (SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1');

View File

@ -2,6 +2,10 @@
-- Regression tests for schemas (namespaces) -- 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 SCHEMA test_ns_schema_1
CREATE UNIQUE INDEX abc_a_idx ON abc (a) CREATE UNIQUE INDEX abc_a_idx ON abc (a)
@ -13,6 +17,26 @@ CREATE SCHEMA test_ns_schema_1
b int UNIQUE 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 -- verify that the objects were created
SELECT COUNT(*) FROM pg_class WHERE relnamespace = SELECT COUNT(*) FROM pg_class WHERE relnamespace =
(SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1'); (SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_1');