diff options
Diffstat (limited to 'src/backend/catalog')
| -rw-r--r-- | src/backend/catalog/pg_collation.c | 49 | ||||
| -rw-r--r-- | src/backend/catalog/pg_depend.c | 77 | ||||
| -rw-r--r-- | src/backend/catalog/pg_operator.c | 2 | ||||
| -rw-r--r-- | src/backend/catalog/pg_type.c | 7 |
4 files changed, 108 insertions, 27 deletions
diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c index 19068b652a0..b1137ca3afb 100644 --- a/src/backend/catalog/pg_collation.c +++ b/src/backend/catalog/pg_collation.c @@ -78,15 +78,25 @@ CollationCreate(const char *collname, Oid collnamespace, * friendlier error message. The unique index provides a backstop against * race conditions. */ - if (SearchSysCacheExists3(COLLNAMEENCNSP, - PointerGetDatum(collname), - Int32GetDatum(collencoding), - ObjectIdGetDatum(collnamespace))) + oid = GetSysCacheOid3(COLLNAMEENCNSP, + Anum_pg_collation_oid, + PointerGetDatum(collname), + Int32GetDatum(collencoding), + ObjectIdGetDatum(collnamespace)); + if (OidIsValid(oid)) { if (quiet) return InvalidOid; else if (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, CollationRelationId, oid); + checkMembershipInCurrentExtension(&myself); + + /* OK to skip */ ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), collencoding == -1 @@ -116,16 +126,19 @@ CollationCreate(const char *collname, Oid collnamespace, * so we take a ShareRowExclusiveLock earlier, to protect against * concurrent changes fooling this check. */ - if ((collencoding == -1 && - SearchSysCacheExists3(COLLNAMEENCNSP, - PointerGetDatum(collname), - Int32GetDatum(GetDatabaseEncoding()), - ObjectIdGetDatum(collnamespace))) || - (collencoding != -1 && - SearchSysCacheExists3(COLLNAMEENCNSP, - PointerGetDatum(collname), - Int32GetDatum(-1), - ObjectIdGetDatum(collnamespace)))) + if (collencoding == -1) + oid = GetSysCacheOid3(COLLNAMEENCNSP, + Anum_pg_collation_oid, + PointerGetDatum(collname), + Int32GetDatum(GetDatabaseEncoding()), + ObjectIdGetDatum(collnamespace)); + else + oid = GetSysCacheOid3(COLLNAMEENCNSP, + Anum_pg_collation_oid, + PointerGetDatum(collname), + Int32GetDatum(-1), + ObjectIdGetDatum(collnamespace)); + if (OidIsValid(oid)) { if (quiet) { @@ -134,6 +147,14 @@ CollationCreate(const char *collname, Oid collnamespace, } else if (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, CollationRelationId, oid); + checkMembershipInCurrentExtension(&myself); + + /* OK to skip */ table_close(rel, NoLock); ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index ae16d2fefa1..07791b47a61 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -166,22 +166,23 @@ recordMultipleDependencies(const ObjectAddress *depender, /* * If we are executing a CREATE EXTENSION operation, mark the given object - * as being a member of the extension. Otherwise, do nothing. + * as being a member of the extension, or check that it already is one. + * Otherwise, do nothing. * * This must be called during creation of any user-definable object type * that could be a member of an extension. * - * If isReplace is true, the object already existed (or might have already - * existed), so we must check for a pre-existing extension membership entry. - * Passing false is a guarantee that the object is newly created, and so - * could not already be a member of any extension. + * isReplace must be true if the object already existed, and false if it is + * newly created. In the former case we insist that it already be a member + * of the current extension. In the latter case we can skip checking whether + * it is already a member of any extension. * * Note: isReplace = true is typically used when updating an object in - * CREATE OR REPLACE and similar commands. The net effect is that if an - * extension script uses such a command on a pre-existing free-standing - * object, the object will be absorbed into the extension. If the object - * is already a member of some other extension, the command will fail. - * This behavior is desirable for cases such as replacing a shell type. + * CREATE OR REPLACE and similar commands. We used to allow the target + * object to not already be an extension member, instead silently absorbing + * it into the current extension. However, this was both error-prone + * (extensions might accidentally overwrite free-standing objects) and + * a security hazard (since the object would retain its previous ownership). */ void recordDependencyOnCurrentExtension(const ObjectAddress *object, @@ -199,6 +200,12 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object, { Oid oldext; + /* + * Side note: these catalog lookups are safe only because the + * object is a pre-existing one. In the not-isReplace case, the + * caller has most likely not yet done a CommandCounterIncrement + * that would make the new object visible. + */ oldext = getExtensionOfObject(object->classId, object->objectId); if (OidIsValid(oldext)) { @@ -212,6 +219,13 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object, getObjectDescription(object, false), get_extension_name(oldext)))); } + /* It's a free-standing object, so reject */ + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("%s is not a member of extension \"%s\"", + getObjectDescription(object, false), + get_extension_name(CurrentExtensionObject)), + errdetail("An extension is not allowed to replace an object that it does not own."))); } /* OK, record it as a member of CurrentExtensionObject */ @@ -224,6 +238,49 @@ recordDependencyOnCurrentExtension(const ObjectAddress *object, } /* + * If we are executing a CREATE EXTENSION operation, check that the given + * object is a member of the extension, and throw an error if it isn't. + * Otherwise, do nothing. + * + * This must be called whenever a CREATE IF NOT EXISTS operation (for an + * object type that can be an extension member) has found that an object of + * the desired name already exists. It is insecure for an extension to use + * IF NOT EXISTS except when the conflicting object is already an extension + * member; otherwise a hostile user could substitute an object with arbitrary + * properties. + */ +void +checkMembershipInCurrentExtension(const ObjectAddress *object) +{ + /* + * This is actually the same condition tested in + * recordDependencyOnCurrentExtension; but we want to issue a + * differently-worded error, and anyway it would be pretty confusing to + * call recordDependencyOnCurrentExtension in these circumstances. + */ + + /* Only whole objects can be extension members */ + Assert(object->objectSubId == 0); + + if (creating_extension) + { + Oid oldext; + + oldext = getExtensionOfObject(object->classId, object->objectId); + /* If already a member of this extension, OK */ + if (oldext == CurrentExtensionObject) + return; + /* Else complain */ + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("%s is not a member of extension \"%s\"", + getObjectDescription(object, false), + get_extension_name(CurrentExtensionObject)), + errdetail("An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns."))); + } +} + +/* * deleteDependencyRecordsFor -- delete all records with given depender * classId/objectId. Returns the number of records deleted. * diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index 4c5a56cb094..5ec3221d533 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -864,7 +864,7 @@ makeOperatorDependencies(HeapTuple tuple, /* Dependency on extension */ if (makeExtensionDep) - recordDependencyOnCurrentExtension(&myself, true); + recordDependencyOnCurrentExtension(&myself, isUpdate); return myself; } diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c index cdce22f394f..b37a8f034be 100644 --- a/src/backend/catalog/pg_type.c +++ b/src/backend/catalog/pg_type.c @@ -548,8 +548,11 @@ TypeCreate(Oid newTypeOid, * rebuild should be true if this is a pre-existing type. We will remove * existing dependencies and rebuild them from scratch. This is needed for * ALTER TYPE, and also when replacing a shell type. We don't remove any - * existing extension dependency, though (hence, if makeExtensionDep is also - * true and the type belongs to some other extension, an error will occur). + * existing extension dependency, though; hence, if makeExtensionDep is also + * true and we're in an extension script, an error will occur unless the + * type already belongs to the current extension. That's the behavior we + * want when replacing a shell type, which is the only case where both flags + * are true. */ void GenerateTypeDependencies(HeapTuple typeTuple, |