From 7bd7b2002bd018e25d024322c983e856237a50d9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 31 Jul 2008 16:27:16 +0000 Subject: [PATCH] Require superuser privilege to create base types (but not composites, enums, or domains). This was already effectively required because you had to own the I/O functions, and the I/O functions pretty much have to be written in C since we don't let PL functions take or return cstring. But given the possible security consequences of a malicious type definition, it seems prudent to enforce superuser requirement directly. Per recent discussion. --- doc/src/sgml/ref/create_type.sgml | 14 ++++++++++---- src/backend/commands/typecmds.c | 27 ++++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml index 665bc805af8..a5d07a21206 100644 --- a/doc/src/sgml/ref/create_type.sgml +++ b/doc/src/sgml/ref/create_type.sgml @@ -1,5 +1,5 @@ @@ -99,7 +99,13 @@ CREATE TYPE name The third form of CREATE TYPE creates a new base type - (scalar type). The parameters can appear in any order, not only that + (scalar type). To create a new base type, you must be a superuser. + (This restriction is made because an erroneous type definition could + confuse or even crash the server.) + + + + The parameters can appear in any order, not only that illustrated above, and most are optional. You must register two or more functions (using CREATE FUNCTION) before defining the type. The support functions @@ -580,8 +586,8 @@ CREATE TYPE name Because there are no restrictions on use of a data type once it's been created, creating a base type is tantamount to granting public execute - permission on the functions mentioned in the type definition. (The creator - of the type is therefore required to own these functions.) This is usually + permission on the functions mentioned in the type definition. + This is usually not an issue for the sorts of functions that are useful in a type definition. But you might want to think twice before designing a type in a way that would require secret information to be used diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 1b3caab2e1b..ee30d32704a 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.121 2008/07/30 19:35:13 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.122 2008/07/31 16:27:16 tgl Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -92,14 +92,13 @@ static char *domainAddConstraint(Oid domainOid, Oid domainNamespace, /* * DefineType - * Registers a new type. + * Registers a new base type. */ void DefineType(List *names, List *parameters) { char *typeName; Oid typeNamespace; - AclResult aclresult; int16 internalLength = -1; /* default: variable-length */ Oid elemType = InvalidOid; List *inputName = NIL; @@ -130,14 +129,33 @@ DefineType(List *names, List *parameters) Oid resulttype; Relation pg_type; + /* + * As of Postgres 8.4, we require superuser privilege to create a base + * type. This is simple paranoia: there are too many ways to mess up the + * system with an incorrect type definition (for instance, representation + * parameters that don't match what the C code expects). In practice + * it takes superuser privilege to create the I/O functions, and so the + * former requirement that you own the I/O functions pretty much forced + * superuserness anyway. We're just making doubly sure here. + * + * XXX re-enable NOT_USED code sections below if you remove this test. + */ + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to create a base type"))); + /* Convert list of names to a name and namespace */ typeNamespace = QualifiedNameGetCreationNamespace(names, &typeName); +#ifdef NOT_USED + /* XXX this is unnecessary given the superuser check above */ /* Check we have creation rights in target namespace */ aclresult = pg_namespace_aclcheck(typeNamespace, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, ACL_KIND_NAMESPACE, get_namespace_name(typeNamespace)); +#endif /* * Look to see if type already exists (presumably as a shell; if not, @@ -398,6 +416,8 @@ DefineType(List *names, List *parameters) * don't have a way to make the type go away if the grant option is * revoked, so ownership seems better. */ +#ifdef NOT_USED + /* XXX this is unnecessary given the superuser check above */ if (inputOid && !pg_proc_ownercheck(inputOid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, NameListToString(inputName)); @@ -419,6 +439,7 @@ DefineType(List *names, List *parameters) if (analyzeOid && !pg_proc_ownercheck(analyzeOid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, NameListToString(analyzeName)); +#endif /* Preassign array type OID so we can insert it in pg_type.typarray */ pg_type = heap_open(TypeRelationId, AccessShareLock);