From bf1e33d24a9611583595eb1c6cc2e7ce3fa01da4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 22 Aug 2005 17:38:20 +0000 Subject: [PATCH] Fix unwanted denial of ALTER OWNER rights to superusers. There was some discussion of getting around this by relaxing the checks made for regular users, but I'm disinclined to toy with the security model right now, so just special-case it for superusers where needed. --- src/backend/commands/aggregatecmds.c | 31 ++++++++++++++---------- src/backend/commands/conversioncmds.c | 31 ++++++++++++++---------- src/backend/commands/dbcommands.c | 5 ++-- src/backend/commands/functioncmds.c | 31 ++++++++++++++---------- src/backend/commands/opclasscmds.c | 30 +++++++++++++---------- src/backend/commands/operatorcmds.c | 31 ++++++++++++++---------- src/backend/commands/schemacmds.c | 5 ++-- src/backend/commands/tablecmds.c | 34 +++++++++++++++------------ src/backend/commands/typecmds.c | 31 ++++++++++++++---------- 9 files changed, 132 insertions(+), 97 deletions(-) diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c index e96f328d19..e3efde249d 100644 --- a/src/backend/commands/aggregatecmds.c +++ b/src/backend/commands/aggregatecmds.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/aggregatecmds.c,v 1.28 2005/07/14 21:46:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/aggregatecmds.c,v 1.29 2005/08/22 17:38:20 tgl Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -332,20 +332,25 @@ AlterAggregateOwner(List *name, TypeName *basetype, Oid newOwnerId) */ if (procForm->proowner != newOwnerId) { - /* Otherwise, must be owner of the existing object */ - if (!pg_proc_ownercheck(procOid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, - NameListToString(name)); + /* Superusers can always do it */ + if (!superuser()) + { + /* Otherwise, must be owner of the existing object */ + if (!pg_proc_ownercheck(procOid, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, + NameListToString(name)); - /* Must be able to become new owner */ - check_is_member_of_role(GetUserId(), newOwnerId); + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); - /* New owner must have CREATE privilege on namespace */ - aclresult = pg_namespace_aclcheck(procForm->pronamespace, newOwnerId, - ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_NAMESPACE, - get_namespace_name(procForm->pronamespace)); + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(procForm->pronamespace, + newOwnerId, + ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(procForm->pronamespace)); + } /* * Modify the owner --- okay to scribble on tup because it's a diff --git a/src/backend/commands/conversioncmds.c b/src/backend/commands/conversioncmds.c index 81e889aa4e..912f35ea20 100644 --- a/src/backend/commands/conversioncmds.c +++ b/src/backend/commands/conversioncmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/conversioncmds.c,v 1.21 2005/07/14 21:46:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/conversioncmds.c,v 1.22 2005/08/22 17:38:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -206,20 +206,25 @@ AlterConversionOwner(List *name, Oid newOwnerId) */ if (convForm->conowner != newOwnerId) { - /* Otherwise, must be owner of the existing object */ - if (!pg_conversion_ownercheck(HeapTupleGetOid(tup),GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CONVERSION, - NameListToString(name)); + /* Superusers can always do it */ + if (!superuser()) + { + /* Otherwise, must be owner of the existing object */ + if (!pg_conversion_ownercheck(HeapTupleGetOid(tup),GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CONVERSION, + NameListToString(name)); - /* Must be able to become new owner */ - check_is_member_of_role(GetUserId(), newOwnerId); + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); - /* New owner must have CREATE privilege on namespace */ - aclresult = pg_namespace_aclcheck(convForm->connamespace, newOwnerId, - ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_NAMESPACE, - get_namespace_name(convForm->connamespace)); + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(convForm->connamespace, + newOwnerId, + ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(convForm->connamespace)); + } /* * Modify the owner --- okay to scribble on tup because it's a diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 40e28a0821..49d3e1d4f5 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -15,7 +15,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.170 2005/08/12 01:35:57 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.171 2005/08/22 17:38:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1024,7 +1024,8 @@ AlterDatabaseOwner(const char *dbname, Oid newOwnerId) * NOTE: This is different from other alter-owner checks in * that the current user is checked for createdb privileges * instead of the destination owner. This is consistent - * with the CREATE case for databases. + * with the CREATE case for databases. Because superusers + * will always have this right, we need no special case for them. */ if (!have_createdb_privilege()) ereport(ERROR, diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 38912b777d..d6bb902274 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/functioncmds.c,v 1.65 2005/08/01 04:03:55 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/functioncmds.c,v 1.66 2005/08/22 17:38:20 tgl Exp $ * * DESCRIPTION * These routines take the parse tree and pick out the @@ -894,20 +894,25 @@ AlterFunctionOwner(List *name, List *argtypes, Oid newOwnerId) bool isNull; HeapTuple newtuple; - /* Otherwise, must be owner of the existing object */ - if (!pg_proc_ownercheck(procOid,GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, - NameListToString(name)); + /* Superusers can always do it */ + if (!superuser()) + { + /* Otherwise, must be owner of the existing object */ + if (!pg_proc_ownercheck(procOid,GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, + NameListToString(name)); - /* Must be able to become new owner */ - check_is_member_of_role(GetUserId(), newOwnerId); + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); - /* New owner must have CREATE privilege on namespace */ - aclresult = pg_namespace_aclcheck(procForm->pronamespace, newOwnerId, - ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_NAMESPACE, - get_namespace_name(procForm->pronamespace)); + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(procForm->pronamespace, + newOwnerId, + ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(procForm->pronamespace)); + } memset(repl_null, ' ', sizeof(repl_null)); memset(repl_repl, ' ', sizeof(repl_repl)); diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c index c9d11da70e..1884c25f17 100644 --- a/src/backend/commands/opclasscmds.c +++ b/src/backend/commands/opclasscmds.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/opclasscmds.c,v 1.35 2005/07/14 21:46:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/opclasscmds.c,v 1.36 2005/08/22 17:38:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -951,20 +951,24 @@ AlterOpClassOwner(List *name, const char *access_method, Oid newOwnerId) */ if (opcForm->opcowner != newOwnerId) { - /* Otherwise, must be owner of the existing object */ - if (!pg_opclass_ownercheck(HeapTupleGetOid(tup),GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_OPCLASS, - NameListToString(name)); + /* Superusers can always do it */ + if (!superuser()) + { + /* Otherwise, must be owner of the existing object */ + if (!pg_opclass_ownercheck(HeapTupleGetOid(tup),GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_OPCLASS, + NameListToString(name)); - /* Must be able to become new owner */ - check_is_member_of_role(GetUserId(), newOwnerId); + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); - /* New owner must have CREATE privilege on namespace */ - aclresult = pg_namespace_aclcheck(namespaceOid, newOwnerId, - ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_NAMESPACE, - get_namespace_name(namespaceOid)); + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(namespaceOid, newOwnerId, + ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(namespaceOid)); + } /* * Modify the owner --- okay to scribble on tup because it's a diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c index d36ac6acd0..f9db742e84 100644 --- a/src/backend/commands/operatorcmds.c +++ b/src/backend/commands/operatorcmds.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/operatorcmds.c,v 1.24 2005/07/14 21:46:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/operatorcmds.c,v 1.25 2005/08/22 17:38:20 tgl Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -296,20 +296,25 @@ AlterOperatorOwner(List *name, TypeName *typeName1, TypeName *typeName2, */ if (oprForm->oprowner != newOwnerId) { - /* Otherwise, must be owner of the existing object */ - if (!pg_oper_ownercheck(operOid,GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_OPER, - NameListToString(name)); + /* Superusers can always do it */ + if (!superuser()) + { + /* Otherwise, must be owner of the existing object */ + if (!pg_oper_ownercheck(operOid,GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_OPER, + NameListToString(name)); - /* Must be able to become new owner */ - check_is_member_of_role(GetUserId(), newOwnerId); + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); - /* New owner must have CREATE privilege on namespace */ - aclresult = pg_namespace_aclcheck(oprForm->oprnamespace, newOwnerId, - ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_NAMESPACE, - get_namespace_name(oprForm->oprnamespace)); + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(oprForm->oprnamespace, + newOwnerId, + ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(oprForm->oprnamespace)); + } /* * Modify the owner --- okay to scribble on tup because it's a diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index 65a6edeabc..f0ae06f15c 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.33 2005/07/14 21:46:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.34 2005/08/22 17:38:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -315,7 +315,8 @@ AlterSchemaOwner(const char *name, Oid newOwnerId) * NOTE: This is different from other alter-owner checks in * that the current user is checked for create privileges * instead of the destination owner. This is consistent - * with the CREATE case for schemas. + * with the CREATE case for schemas. Because superusers + * will always have this right, we need no special case for them. */ aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(), ACL_CREATE); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index aaf9a2ce74..95e3ef68bb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.166 2005/08/04 01:09:28 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.167 2005/08/22 17:38:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -5307,23 +5307,27 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing) /* skip permission checks when recursing to index or toast table */ if (!recursing) { - Oid namespaceOid = tuple_class->relnamespace; - AclResult aclresult; + /* Superusers can always do it */ + if (!superuser()) + { + Oid namespaceOid = tuple_class->relnamespace; + AclResult aclresult; - /* Otherwise, must be owner of the existing object */ - if (!pg_class_ownercheck(relationOid,GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, - RelationGetRelationName(target_rel)); + /* Otherwise, must be owner of the existing object */ + if (!pg_class_ownercheck(relationOid,GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, + RelationGetRelationName(target_rel)); - /* Must be able to become new owner */ - check_is_member_of_role(GetUserId(), newOwnerId); + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); - /* New owner must have CREATE privilege on namespace */ - aclresult = pg_namespace_aclcheck(namespaceOid, newOwnerId, - ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_NAMESPACE, - get_namespace_name(namespaceOid)); + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(namespaceOid, newOwnerId, + ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(namespaceOid)); + } } memset(repl_null, ' ', sizeof(repl_null)); diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index e0c3a311ea..ee69821bcf 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.79 2005/08/12 01:35:58 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.80 2005/08/22 17:38:20 tgl Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -2067,20 +2067,25 @@ AlterTypeOwner(List *names, Oid newOwnerId) */ if (typTup->typowner != newOwnerId) { - /* Otherwise, must be owner of the existing object */ - if (!pg_type_ownercheck(HeapTupleGetOid(tup),GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - TypeNameToString(typename)); + /* Superusers can always do it */ + if (!superuser()) + { + /* Otherwise, must be owner of the existing object */ + if (!pg_type_ownercheck(HeapTupleGetOid(tup),GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, + TypeNameToString(typename)); - /* Must be able to become new owner */ - check_is_member_of_role(GetUserId(), newOwnerId); + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); - /* New owner must have CREATE privilege on namespace */ - aclresult = pg_namespace_aclcheck(typTup->typnamespace, newOwnerId, - ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_NAMESPACE, - get_namespace_name(typTup->typnamespace)); + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(typTup->typnamespace, + newOwnerId, + ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(typTup->typnamespace)); + } /* * Modify the owner --- okay to scribble on typTup because it's a