Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite8ad98b

Browse files
tglsfdcpull[bot]
authored andcommitted
In extensions, don't replace objects not belonging to the extension.
Previously, if an extension script did CREATE OR REPLACE and there wasan existing object not belonging to the extension, it would overwritethe object and adopt it into the extension. This is problematic, firstbecause the overwrite is probably unintentional, and second because wedidn't change the object's ownership. Thus a hostile user could createan object in advance of an expected CREATE EXTENSION command, and wouldthen have ownership rights on an extension object, which could bemodified for trojan-horse-type attacks.Hence, forbid CREATE OR REPLACE of an existing object unless it alreadybelongs to the extension. (Note that we've always forbidden replacingan object that belongs to some other extension; only the behavior forpreviously-free-standing objects changes here.)For the same reason, also fail CREATE IF NOT EXISTS when there isan existing object that doesn't belong to the extension.Our thanks to Sven Klemm for reporting this problem.Security:CVE-2022-2625
1 parentc0c301a commite8ad98b

21 files changed

+539
-52
lines changed

‎doc/src/sgml/extend.sgml

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,17 +1319,6 @@ SELECT * FROM pg_extension_update_paths('<replaceable>extension_name</replaceabl
13191319
trusted if it depends on another one, unless that other one is always
13201320
installed in <literal>pg_catalog</literal>.
13211321
</para>
1322-
1323-
<para>
1324-
Do <emphasis>not</emphasis> use <command>CREATE OR REPLACE
1325-
FUNCTION</command>, except in an update script that must change the
1326-
definition of a function that is known to be an extension member
1327-
already. (Likewise for other <literal>OR REPLACE</literal> options.)
1328-
Using <literal>OR REPLACE</literal> unnecessarily not only has a risk
1329-
of accidentally overwriting someone else's function, but it creates a
1330-
security hazard since the overwritten function would still be owned by
1331-
its original owner, who could modify it.
1332-
</para>
13331322
</sect3>
13341323
</sect2>
13351324

‎src/backend/catalog/pg_collation.c

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,25 @@ CollationCreate(const char *collname, Oid collnamespace,
7676
* friendlier error message. The unique index provides a backstop against
7777
* race conditions.
7878
*/
79-
if (SearchSysCacheExists3(COLLNAMEENCNSP,
80-
PointerGetDatum(collname),
81-
Int32GetDatum(collencoding),
82-
ObjectIdGetDatum(collnamespace)))
79+
oid=GetSysCacheOid3(COLLNAMEENCNSP,
80+
Anum_pg_collation_oid,
81+
PointerGetDatum(collname),
82+
Int32GetDatum(collencoding),
83+
ObjectIdGetDatum(collnamespace));
84+
if (OidIsValid(oid))
8385
{
8486
if (quiet)
8587
returnInvalidOid;
8688
elseif (if_not_exists)
8789
{
90+
/*
91+
* If we are in an extension script, insist that the pre-existing
92+
* object be a member of the extension, to avoid security risks.
93+
*/
94+
ObjectAddressSet(myself,CollationRelationId,oid);
95+
checkMembershipInCurrentExtension(&myself);
96+
97+
/* OK to skip */
8898
ereport(NOTICE,
8999
(errcode(ERRCODE_DUPLICATE_OBJECT),
90100
collencoding==-1
@@ -114,16 +124,19 @@ CollationCreate(const char *collname, Oid collnamespace,
114124
* so we take a ShareRowExclusiveLock earlier, to protect against
115125
* concurrent changes fooling this check.
116126
*/
117-
if ((collencoding==-1&&
118-
SearchSysCacheExists3(COLLNAMEENCNSP,
119-
PointerGetDatum(collname),
120-
Int32GetDatum(GetDatabaseEncoding()),
121-
ObjectIdGetDatum(collnamespace)))||
122-
(collencoding!=-1&&
123-
SearchSysCacheExists3(COLLNAMEENCNSP,
124-
PointerGetDatum(collname),
125-
Int32GetDatum(-1),
126-
ObjectIdGetDatum(collnamespace))))
127+
if (collencoding==-1)
128+
oid=GetSysCacheOid3(COLLNAMEENCNSP,
129+
Anum_pg_collation_oid,
130+
PointerGetDatum(collname),
131+
Int32GetDatum(GetDatabaseEncoding()),
132+
ObjectIdGetDatum(collnamespace));
133+
else
134+
oid=GetSysCacheOid3(COLLNAMEENCNSP,
135+
Anum_pg_collation_oid,
136+
PointerGetDatum(collname),
137+
Int32GetDatum(-1),
138+
ObjectIdGetDatum(collnamespace));
139+
if (OidIsValid(oid))
127140
{
128141
if (quiet)
129142
{
@@ -132,6 +145,14 @@ CollationCreate(const char *collname, Oid collnamespace,
132145
}
133146
elseif (if_not_exists)
134147
{
148+
/*
149+
* If we are in an extension script, insist that the pre-existing
150+
* object be a member of the extension, to avoid security risks.
151+
*/
152+
ObjectAddressSet(myself,CollationRelationId,oid);
153+
checkMembershipInCurrentExtension(&myself);
154+
155+
/* OK to skip */
135156
table_close(rel,NoLock);
136157
ereport(NOTICE,
137158
(errcode(ERRCODE_DUPLICATE_OBJECT),

‎src/backend/catalog/pg_depend.c

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -170,22 +170,23 @@ recordMultipleDependencies(const ObjectAddress *depender,
170170

171171
/*
172172
* If we are executing a CREATE EXTENSION operation, mark the given object
173-
* as being a member of the extension. Otherwise, do nothing.
173+
* as being a member of the extension, or check that it already is one.
174+
* Otherwise, do nothing.
174175
*
175176
* This must be called during creation of any user-definable object type
176177
* that could be a member of an extension.
177178
*
178-
*IfisReplaceistrue,the object already existed (or might have already
179-
*existed), so we must check for a pre-existing extension membership entry.
180-
*Passing false is a guarantee that theobject is newly created, and so
181-
*could not already be a member of any extension.
179+
* isReplacemust betrue ifthe object already existed, and false if it is
180+
*newly created. In the former case we insist that it already be a member
181+
*of the current extension. In thelatter case we can skip checking whether
182+
*it is already a member of any extension.
182183
*
183184
* Note: isReplace = true is typically used when updating an object in
184-
* CREATE OR REPLACE and similar commands.The net effect is that if an
185-
*extension script uses such a command on a pre-existing free-standing
186-
*object, the object will be absorbedinto the extension.If the object
187-
*is already a member of some other extension, the command will fail.
188-
*This behavior is desirable for cases such as replacing a shell type.
185+
* CREATE OR REPLACE and similar commands.We used to allow the target
186+
*object to not already be an extension member, instead silently absorbing
187+
*itinto thecurrentextension.However, this was both error-prone
188+
*(extensions might accidentally overwrite free-standing objects) and
189+
*a security hazard (since the object would retain its previous ownership).
189190
*/
190191
void
191192
recordDependencyOnCurrentExtension(constObjectAddress*object,
@@ -203,6 +204,12 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object,
203204
{
204205
Oidoldext;
205206

207+
/*
208+
* Side note: these catalog lookups are safe only because the
209+
* object is a pre-existing one. In the not-isReplace case, the
210+
* caller has most likely not yet done a CommandCounterIncrement
211+
* that would make the new object visible.
212+
*/
206213
oldext=getExtensionOfObject(object->classId,object->objectId);
207214
if (OidIsValid(oldext))
208215
{
@@ -216,6 +223,13 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object,
216223
getObjectDescription(object, false),
217224
get_extension_name(oldext))));
218225
}
226+
/* It's a free-standing object, so reject */
227+
ereport(ERROR,
228+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
229+
errmsg("%s is not a member of extension \"%s\"",
230+
getObjectDescription(object, false),
231+
get_extension_name(CurrentExtensionObject)),
232+
errdetail("An extension is not allowed to replace an object that it does not own.")));
219233
}
220234

221235
/* OK, record it as a member of CurrentExtensionObject */
@@ -227,6 +241,49 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object,
227241
}
228242
}
229243

244+
/*
245+
* If we are executing a CREATE EXTENSION operation, check that the given
246+
* object is a member of the extension, and throw an error if it isn't.
247+
* Otherwise, do nothing.
248+
*
249+
* This must be called whenever a CREATE IF NOT EXISTS operation (for an
250+
* object type that can be an extension member) has found that an object of
251+
* the desired name already exists. It is insecure for an extension to use
252+
* IF NOT EXISTS except when the conflicting object is already an extension
253+
* member; otherwise a hostile user could substitute an object with arbitrary
254+
* properties.
255+
*/
256+
void
257+
checkMembershipInCurrentExtension(constObjectAddress*object)
258+
{
259+
/*
260+
* This is actually the same condition tested in
261+
* recordDependencyOnCurrentExtension; but we want to issue a
262+
* differently-worded error, and anyway it would be pretty confusing to
263+
* call recordDependencyOnCurrentExtension in these circumstances.
264+
*/
265+
266+
/* Only whole objects can be extension members */
267+
Assert(object->objectSubId==0);
268+
269+
if (creating_extension)
270+
{
271+
Oidoldext;
272+
273+
oldext=getExtensionOfObject(object->classId,object->objectId);
274+
/* If already a member of this extension, OK */
275+
if (oldext==CurrentExtensionObject)
276+
return;
277+
/* Else complain */
278+
ereport(ERROR,
279+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
280+
errmsg("%s is not a member of extension \"%s\"",
281+
getObjectDescription(object, false),
282+
get_extension_name(CurrentExtensionObject)),
283+
errdetail("An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.")));
284+
}
285+
}
286+
230287
/*
231288
* deleteDependencyRecordsFor -- delete all records with given depender
232289
* classId/objectId. Returns the number of records deleted.

‎src/backend/catalog/pg_operator.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ makeOperatorDependencies(HeapTuple tuple,
864864

865865
/* Dependency on extension */
866866
if (makeExtensionDep)
867-
recordDependencyOnCurrentExtension(&myself,true);
867+
recordDependencyOnCurrentExtension(&myself,isUpdate);
868868

869869
returnmyself;
870870
}

‎src/backend/catalog/pg_type.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,8 +546,11 @@ TypeCreate(Oid newTypeOid,
546546
* rebuild should be true if this is a pre-existing type. We will remove
547547
* existing dependencies and rebuild them from scratch. This is needed for
548548
* ALTER TYPE, and also when replacing a shell type. We don't remove any
549-
* existing extension dependency, though (hence, if makeExtensionDep is also
550-
* true and the type belongs to some other extension, an error will occur).
549+
* existing extension dependency, though; hence, if makeExtensionDep is also
550+
* true and we're in an extension script, an error will occur unless the
551+
* type already belongs to the current extension. That's the behavior we
552+
* want when replacing a shell type, which is the only case where both flags
553+
* are true.
551554
*/
552555
void
553556
GenerateTypeDependencies(HeapTupletypeTuple,

‎src/backend/commands/createas.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,19 +393,31 @@ bool
393393
CreateTableAsRelExists(CreateTableAsStmt*ctas)
394394
{
395395
Oidnspid;
396+
Oidoldrelid;
397+
ObjectAddressaddress;
396398
IntoClause*into=ctas->into;
397399

398400
nspid=RangeVarGetCreationNamespace(into->rel);
399401

400-
if (get_relname_relid(into->rel->relname,nspid))
402+
oldrelid=get_relname_relid(into->rel->relname,nspid);
403+
if (OidIsValid(oldrelid))
401404
{
402405
if (!ctas->if_not_exists)
403406
ereport(ERROR,
404407
(errcode(ERRCODE_DUPLICATE_TABLE),
405408
errmsg("relation \"%s\" already exists",
406409
into->rel->relname)));
407410

408-
/* The relation exists and IF NOT EXISTS has been specified */
411+
/*
412+
* The relation exists and IF NOT EXISTS has been specified.
413+
*
414+
* If we are in an extension script, insist that the pre-existing
415+
* object be a member of the extension, to avoid security risks.
416+
*/
417+
ObjectAddressSet(address,RelationRelationId,oldrelid);
418+
checkMembershipInCurrentExtension(&address);
419+
420+
/* OK to skip */
409421
ereport(NOTICE,
410422
(errcode(ERRCODE_DUPLICATE_TABLE),
411423
errmsg("relation \"%s\" already exists, skipping",

‎src/backend/commands/foreigncmds.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -855,13 +855,22 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
855855
ownerId=GetUserId();
856856

857857
/*
858-
* Check that there is no other foreign server by this name.Do nothing if
859-
* IF NOT EXISTS wasenforced.
858+
* Check that there is no other foreign server by this name. If there is
859+
*one, do nothing ifIF NOT EXISTS wasspecified.
860860
*/
861-
if (GetForeignServerByName(stmt->servername, true)!=NULL)
861+
srvId=get_foreign_server_oid(stmt->servername, true);
862+
if (OidIsValid(srvId))
862863
{
863864
if (stmt->if_not_exists)
864865
{
866+
/*
867+
* If we are in an extension script, insist that the pre-existing
868+
* object be a member of the extension, to avoid security risks.
869+
*/
870+
ObjectAddressSet(myself,ForeignServerRelationId,srvId);
871+
checkMembershipInCurrentExtension(&myself);
872+
873+
/* OK to skip */
865874
ereport(NOTICE,
866875
(errcode(ERRCODE_DUPLICATE_OBJECT),
867876
errmsg("server \"%s\" already exists, skipping",
@@ -1126,6 +1135,10 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
11261135
{
11271136
if (stmt->if_not_exists)
11281137
{
1138+
/*
1139+
* Since user mappings aren't members of extensions (see comments
1140+
* below), no need for checkMembershipInCurrentExtension here.
1141+
*/
11291142
ereport(NOTICE,
11301143
(errcode(ERRCODE_DUPLICATE_OBJECT),
11311144
errmsg("user mapping for \"%s\" already exists for server \"%s\", skipping",

‎src/backend/commands/schemacmds.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,25 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
112112
* the permissions checks, but since CREATE TABLE IF NOT EXISTS makes its
113113
* creation-permission check first, we do likewise.
114114
*/
115-
if (stmt->if_not_exists&&
116-
SearchSysCacheExists1(NAMESPACENAME,PointerGetDatum(schemaName)))
115+
if (stmt->if_not_exists)
117116
{
118-
ereport(NOTICE,
119-
(errcode(ERRCODE_DUPLICATE_SCHEMA),
120-
errmsg("schema \"%s\" already exists, skipping",
121-
schemaName)));
122-
returnInvalidOid;
117+
namespaceId=get_namespace_oid(schemaName, true);
118+
if (OidIsValid(namespaceId))
119+
{
120+
/*
121+
* If we are in an extension script, insist that the pre-existing
122+
* object be a member of the extension, to avoid security risks.
123+
*/
124+
ObjectAddressSet(address,NamespaceRelationId,namespaceId);
125+
checkMembershipInCurrentExtension(&address);
126+
127+
/* OK to skip */
128+
ereport(NOTICE,
129+
(errcode(ERRCODE_DUPLICATE_SCHEMA),
130+
errmsg("schema \"%s\" already exists, skipping",
131+
schemaName)));
132+
returnInvalidOid;
133+
}
123134
}
124135

125136
/*

‎src/backend/commands/sequence.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,14 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
145145
RangeVarGetAndCheckCreationNamespace(seq->sequence,NoLock,&seqoid);
146146
if (OidIsValid(seqoid))
147147
{
148+
/*
149+
* If we are in an extension script, insist that the pre-existing
150+
* object be a member of the extension, to avoid security risks.
151+
*/
152+
ObjectAddressSet(address,RelationRelationId,seqoid);
153+
checkMembershipInCurrentExtension(&address);
154+
155+
/* OK to skip */
148156
ereport(NOTICE,
149157
(errcode(ERRCODE_DUPLICATE_TABLE),
150158
errmsg("relation \"%s\" already exists, skipping",

‎src/backend/commands/statscmds.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ CreateStatistics(CreateStatsStmt *stmt)
181181
{
182182
if (stmt->if_not_exists)
183183
{
184+
/*
185+
* Since stats objects aren't members of extensions (see comments
186+
* below), no need for checkMembershipInCurrentExtension here.
187+
*/
184188
ereport(NOTICE,
185189
(errcode(ERRCODE_DUPLICATE_OBJECT),
186190
errmsg("statistics object \"%s\" already exists, skipping",

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp