We allow non-superuser database owners to execute CREATE LANGUAGE for a trusted language (one marked as tmpldbacreate in pg_pltemplate). Currently, the C-language support functions for the language end up owned by that non-superuser. This is on the hairy edge of being a security hole, since generally it's supposed that a function owner can redefine the function. Could the non-superuser alter the function into a state where it can be used unsafely? A non-superuser cannot directly execute CREATE OR REPLACE FUNCTION with LANGUAGE set to C, but what can he do if he owns a function that already has that setting?
One possible attack path is to use ALTER FUNCTION RENAME, which with a C language function might be thought to change the target entry point in the language's shared library, thus leading at least to a server crash and possibly to undesirable execution of C-level code. But actually that won't happen, because the target is identified by pg_proc.prosrc which is set up from the function name at CREATE time and isn't changed by RENAME. Besides, the functions at issue here are created in the pg_catalog schema, and non-superusers do not have permission to rename anything in pg_catalog. One thing the owner *can* do is use ALTER FUNCTION to change secondary properties of the function, such as strictness, volatility, SECURITY DEFINER, etc. So far as I can see, none of these properties are examined for a PL support function when it is used to call or validate a function in the language, so this doesn't constitute a security hole either. Still, it's not very hard to envision innocent-looking extensions to ALTER FUNCTION that might result in live security holes here. A different line of attack is to replace the function altogether with CREATE OR REPLACE FUNCTION, using a non-C-language definition. It would no longer be a gateway to executing non-SQL code ... but it would still be referenced by the PL. In this way the function owner could insert trojan-horse code that would be executed whenever somebody else tried to define or use a function written in the PL. It turns out that the placement of the support functions in pg_catalog saves us from this too, but that seems rather accidental to me; it's not immediately obvious that a REPLACE operation on an existing object should require CREATE rights on the containing schema. In short, neither I nor anybody else on the PG security list have been able to think of an exploitable security issue here, but we're not entirely convinced that there isn't one. Can anyone think of an attack vector we missed? As of 9.2, there is a new hazard of the same ilk, namely the constructor functions for range types, which are language INTERNAL and are being created as owned by the type's creator. This seems potentially a worse hole than the CREATE LANGUAGE case, in that any random SQL user can create a range type, not only the database owner (who might be assumed to be at least somewhat trustworthy). Furthermore, because these functions aren't created in pg_catalog but in the type's creation schema, the protections afforded to functions in pg_catalog don't help us. I still don't see any exploitable security hole from ALTER FUNCTION, but it is definitely possible for a range type's creator to replace a constructor function with a trojan horse. The significance of that is debatable though, since you more or less have to trust a type's creator anyway if you use any of its functions. (An example is that a domain's creator can trivially insert trojan horse functions into the domain's CHECK constraints.) Whether or not there is a live security hole in existing releases, it seems clear that we could easily create one by accident in future. To forestall that, I suggest that we should modify CREATE LANGUAGE and CREATE RANGE TYPE to mark the support functions as owned by the bootstrap superuser, not the caller of the CREATE operation. This would ensure that non-superusers couldn't muck around with the function definitions. (Dropping the language or type still works, since cascade deletions don't pay attention to who owns an object that the delete cascades to.) At present it seems sufficient to patch this in HEAD, along the lines of the attached proposed patch. If anyone can think of a workable attack against existing releases, we will need to back-patch some form of this change, and also advise DBAs to manually alter the ownership of existing language support functions. That would be enough of a pain in the rear that I don't want to counsel DBAs to do it unless there's a demonstrable need. Another point here is that if we replace the current pg_pltemplate-based method of creating trusted languages, we will need to be sure that the created C functions always end up owned by a superuser, else the problem comes back again. That will be a matter to consider when we think about how CREATE EXTENSION works for that case. Comments? regards, tom lane
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 9ff70a52e6c7f7d6a6d127a050055320c459b453..0b393cf6ceeaa668c65b00bdb50533b973a91b74 100644 *** a/src/backend/catalog/pg_aggregate.c --- b/src/backend/catalog/pg_aggregate.c *************** AggregateCreate(const char *aggName, *** 233,238 **** --- 233,239 ---- false, /* no replacement */ false, /* doesn't return a set */ finaltype, /* returnType */ + GetUserId(), /* proowner */ INTERNALlanguageId, /* languageObjectId */ InvalidOid, /* no validator */ "aggregate_dummy", /* placeholder proc */ diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 1fffe1c6ac313717191fd9427dbcb2ba8af2da15..ae71b939175cd3682c503f4b44c9eeda580d57c4 100644 *** a/src/backend/catalog/pg_proc.c --- b/src/backend/catalog/pg_proc.c *************** ProcedureCreate(const char *procedureNam *** 69,74 **** --- 69,75 ---- bool replace, bool returnsSet, Oid returnType, + Oid proowner, Oid languageObjectId, Oid languageValidator, const char *prosrc, *************** ProcedureCreate(const char *procedureNam *** 100,106 **** bool internalInParam = false; bool internalOutParam = false; Oid variadicType = InvalidOid; - Oid proowner = GetUserId(); Acl *proacl = NULL; Relation rel; HeapTuple tup; --- 101,106 ---- diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 5f1c19eb3755c52cebd9082cb3e5b3d1f426544a..ff0836c141e25a91aba9308092cd1f0f15ec8768 100644 *** a/src/backend/commands/functioncmds.c --- b/src/backend/commands/functioncmds.c *************** CreateFunction(CreateFunctionStmt *stmt, *** 978,983 **** --- 978,984 ---- stmt->replace, returnsSet, prorettype, + GetUserId(), languageOid, languageValidator, prosrc_str, /* converted to text later */ diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c index 41775fd86745a4538c2e26689e7cbf5ef56f8c26..5d2e7dc1951ac26f3c7f2c9809e76e7dcc75fc57 100644 *** a/src/backend/commands/proclang.c --- b/src/backend/commands/proclang.c *************** *** 18,23 **** --- 18,24 ---- #include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/objectaccess.h" + #include "catalog/pg_authid.h" #include "catalog/pg_language.h" #include "catalog/pg_namespace.h" #include "catalog/pg_pltemplate.h" *************** CreateProceduralLanguage(CreatePLangStmt *** 124,129 **** --- 125,131 ---- false, /* replace */ false, /* returnsSet */ LANGUAGE_HANDLEROID, + BOOTSTRAP_SUPERUSERID, ClanguageId, F_FMGR_C_VALIDATOR, pltemplate->tmplhandler, *************** CreateProceduralLanguage(CreatePLangStmt *** 160,165 **** --- 162,168 ---- false, /* replace */ false, /* returnsSet */ VOIDOID, + BOOTSTRAP_SUPERUSERID, ClanguageId, F_FMGR_C_VALIDATOR, pltemplate->tmplinline, *************** CreateProceduralLanguage(CreatePLangStmt *** 199,204 **** --- 202,208 ---- false, /* replace */ false, /* returnsSet */ VOIDOID, + BOOTSTRAP_SUPERUSERID, ClanguageId, F_FMGR_C_VALIDATOR, pltemplate->tmplvalidator, diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 140b3f851e9ec92181871ce3a32d2b1ba2873302..77559842e5301f2b31596242304ee7f78ee17afd 100644 *** a/src/backend/commands/typecmds.c --- b/src/backend/commands/typecmds.c *************** *** 38,43 **** --- 38,44 ---- #include "catalog/dependency.h" #include "catalog/heap.h" #include "catalog/indexing.h" + #include "catalog/pg_authid.h" #include "catalog/pg_collation.h" #include "catalog/pg_constraint.h" #include "catalog/pg_depend.h" *************** makeRangeConstructors(const char *name, *** 1513,1518 **** --- 1514,1520 ---- false, /* replace */ false, /* returns set */ rangeOid, /* return type */ + BOOTSTRAP_SUPERUSERID, /* proowner */ INTERNALlanguageId, /* language */ F_FMGR_INTERNAL_VALIDATOR, /* language validator */ prosrc[i], /* prosrc */ diff --git a/src/include/catalog/pg_proc_fn.h b/src/include/catalog/pg_proc_fn.h index 04fa64519dac698c9515e62ef82383089c8ed56f..fcd09e7059c9db475185f344e9bf3bb9b4b09226 100644 *** a/src/include/catalog/pg_proc_fn.h --- b/src/include/catalog/pg_proc_fn.h *************** extern Oid ProcedureCreate(const char *p *** 21,26 **** --- 21,27 ---- bool replace, bool returnsSet, Oid returnType, + Oid proowner, Oid languageObjectId, Oid languageValidator, const char *prosrc,
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers