Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane2022-08-08 15:12:31 +0000
committerTom Lane2022-08-08 15:12:31 +0000
commitb9b21acc766db54d8c337d508d0fe2f5bf2daab0 (patch)
treef1e302dbebbfb0ec7cc53f9bbfbd3ef91463c481 /src/backend/commands
parent7e29a79a46d30dc236d097825ab849158929d977 (diff)
In extensions, don't replace objects not belonging to the extension.
Previously, if an extension script did CREATE OR REPLACE and there was an existing object not belonging to the extension, it would overwrite the object and adopt it into the extension. This is problematic, first because the overwrite is probably unintentional, and second because we didn't change the object's ownership. Thus a hostile user could create an object in advance of an expected CREATE EXTENSION command, and would then have ownership rights on an extension object, which could be modified for trojan-horse-type attacks. Hence, forbid CREATE OR REPLACE of an existing object unless it already belongs to the extension. (Note that we've always forbidden replacing an object that belongs to some other extension; only the behavior for previously-free-standing objects changes here.) For the same reason, also fail CREATE IF NOT EXISTS when there is an existing object that doesn't belong to the extension. Our thanks to Sven Klemm for reporting this problem. Security: CVE-2022-2625
Diffstat (limited to 'src/backend/commands')
-rw-r--r--src/backend/commands/createas.c16
-rw-r--r--src/backend/commands/foreigncmds.c19
-rw-r--r--src/backend/commands/schemacmds.c25
-rw-r--r--src/backend/commands/sequence.c8
-rw-r--r--src/backend/commands/statscmds.c4
-rw-r--r--src/backend/commands/view.c16
6 files changed, 75 insertions, 13 deletions
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 9abbb6b5552..152c29b551a 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -393,11 +393,14 @@ bool
CreateTableAsRelExists(CreateTableAsStmt *ctas)
{
Oid nspid;
+ Oid oldrelid;
+ ObjectAddress address;
IntoClause *into = ctas->into;
nspid = RangeVarGetCreationNamespace(into->rel);
- if (get_relname_relid(into->rel->relname, nspid))
+ oldrelid = get_relname_relid(into->rel->relname, nspid);
+ if (OidIsValid(oldrelid))
{
if (!ctas->if_not_exists)
ereport(ERROR,
@@ -405,7 +408,16 @@ CreateTableAsRelExists(CreateTableAsStmt *ctas)
errmsg("relation \"%s\" already exists",
into->rel->relname)));
- /* The relation exists and IF NOT EXISTS has been specified */
+ /*
+ * The relation exists and IF NOT EXISTS has been specified.
+ *
+ * If we are in an extension script, insist that the pre-existing
+ * object be a member of the extension, to avoid security risks.
+ */
+ ObjectAddressSet(address, RelationRelationId, oldrelid);
+ checkMembershipInCurrentExtension(&address);
+
+ /* OK to skip */
ereport(NOTICE,
(errcode(ERRCODE_DUPLICATE_TABLE),
errmsg("relation \"%s\" already exists, skipping",
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index ea27857bb84..91f4dd30de1 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -855,13 +855,22 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
ownerId = GetUserId();
/*
- * Check that there is no other foreign server by this name. Do nothing if
- * IF NOT EXISTS was enforced.
+ * Check that there is no other foreign server by this name. If there is
+ * one, do nothing if IF NOT EXISTS was specified.
*/
- if (GetForeignServerByName(stmt->servername, true) != NULL)
+ srvId = get_foreign_server_oid(stmt->servername, true);
+ if (OidIsValid(srvId))
{
if (stmt->if_not_exists)
{
+ /*
+ * If we are in an extension script, insist that the pre-existing
+ * object be a member of the extension, to avoid security risks.
+ */
+ ObjectAddressSet(myself, ForeignServerRelationId, srvId);
+ checkMembershipInCurrentExtension(&myself);
+
+ /* OK to skip */
ereport(NOTICE,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("server \"%s\" already exists, skipping",
@@ -1126,6 +1135,10 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
{
if (stmt->if_not_exists)
{
+ /*
+ * Since user mappings aren't members of extensions (see comments
+ * below), no need for checkMembershipInCurrentExtension here.
+ */
ereport(NOTICE,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("user mapping for \"%s\" already exists for server \"%s\", skipping",
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index be3925b3b45..a583aa4304b 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -112,14 +112,25 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
* the permissions checks, but since CREATE TABLE IF NOT EXISTS makes its
* creation-permission check first, we do likewise.
*/
- if (stmt->if_not_exists &&
- SearchSysCacheExists1(NAMESPACENAME, PointerGetDatum(schemaName)))
+ if (stmt->if_not_exists)
{
- ereport(NOTICE,
- (errcode(ERRCODE_DUPLICATE_SCHEMA),
- errmsg("schema \"%s\" already exists, skipping",
- schemaName)));
- return InvalidOid;
+ namespaceId = get_namespace_oid(schemaName, true);
+ if (OidIsValid(namespaceId))
+ {
+ /*
+ * If we are in an extension script, insist that the pre-existing
+ * object be a member of the extension, to avoid security risks.
+ */
+ ObjectAddressSet(address, NamespaceRelationId, namespaceId);
+ checkMembershipInCurrentExtension(&address);
+
+ /* OK to skip */
+ ereport(NOTICE,
+ (errcode(ERRCODE_DUPLICATE_SCHEMA),
+ errmsg("schema \"%s\" already exists, skipping",
+ schemaName)));
+ return InvalidOid;
+ }
}
/*
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index b0b211891c3..99c9f91cba5 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -145,6 +145,14 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
RangeVarGetAndCheckCreationNamespace(seq->sequence, NoLock, &seqoid);
if (OidIsValid(seqoid))
{
+ /*
+ * If we are in an extension script, insist that the pre-existing
+ * object be a member of the extension, to avoid security risks.
+ */
+ ObjectAddressSet(address, RelationRelationId, seqoid);
+ checkMembershipInCurrentExtension(&address);
+
+ /* OK to skip */
ereport(NOTICE,
(errcode(ERRCODE_DUPLICATE_TABLE),
errmsg("relation \"%s\" already exists, skipping",
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 415016969dd..7c62bebfd2d 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -181,6 +181,10 @@ CreateStatistics(CreateStatsStmt *stmt)
{
if (stmt->if_not_exists)
{
+ /*
+ * Since stats objects aren't members of extensions (see comments
+ * below), no need for checkMembershipInCurrentExtension here.
+ */
ereport(NOTICE,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("statistics object \"%s\" already exists, skipping",
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 8690a3f3c64..b5a0fc02e5c 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -190,7 +190,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
CommandCounterIncrement();
/*
- * Finally update the view options.
+ * Update the view's options.
*
* The new options list replaces the existing options list, even if
* it's empty.
@@ -203,8 +203,22 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
/* EventTriggerAlterTableStart called by ProcessUtilitySlow */
AlterTableInternal(viewOid, atcmds, true);
+ /*
+ * There is very little to do here to update the view's dependencies.
+ * Most view-level dependency relationships, such as those on the
+ * owner, schema, and associated composite type, aren't changing.
+ * Because we don't allow changing type or collation of an existing
+ * view column, those dependencies of the existing columns don't
+ * change either, while the AT_AddColumnToView machinery took care of
+ * adding such dependencies for new view columns. The dependencies of
+ * the view's query could have changed arbitrarily, but that was dealt
+ * with inside StoreViewQuery. What remains is only to check that
+ * view replacement is allowed when we're creating an extension.
+ */
ObjectAddressSet(address, RelationRelationId, viewOid);
+ recordDependencyOnCurrentExtension(&address, true);
+
/*
* Seems okay, so return the OID of the pre-existing view.
*/