On Wed, May 30, 2012 at 07:34:16PM -0400, Noah Misch wrote:
> ALTER FUNCTION OWNER TO on a C-language function conveys more trust than
> meets the eye:
>
> BEGIN;
> CREATE ROLE alice;
> CREATE FUNCTION mylen(text) RETURNS integer LANGUAGE internal IMMUTABLE
> STRICT AS 'textlen';
> ALTER FUNCTION mylen(text) OWNER TO alice;
> COMMIT;
>
> SET SESSION AUTHORIZATION alice;
> ALTER FUNCTION mylen(text) CALLED ON NULL INPUT;
> SELECT mylen(NULL); -- SIGSEGV
>
> CREATE FUNCTION + ALTER FUNCTION OWNER TO is useful for creating another
> user's untrusted-language SECURITY DEFINER function. ALTER FUNCTION CALLED ON
> NULL INPUT ought to require that the user be eligible to redefine the function
> completely.
Here's a patch implementing that restriction. To clarify, I see no need to
repeat *all* the CREATE-time checks; for example, there's no need to recheck
permission to use the return type. The language usage check is enough.
I didn't feel the need to memorialize a test like the above in an actual
regression test, but that's the one I used to verify the change.
Considering the crash potential, I'd recommend backpatching this.
Thanks,
nm
diff --git a/doc/src/sgml/ref/alter_function.sgml
b/doc/src/sgml/ref/alter_function.sgml
index 013b6f8..cfc2a69 100644
*** a/doc/src/sgml/ref/alter_function.sgml
--- b/doc/src/sgml/ref/alter_function.sgml
***************
*** 54,59 **** ALTER FUNCTION <replaceable>name</replaceable> ( [ [
<replaceable class="paramet
--- 54,61 ----
<para>
You must own the function to use <command>ALTER FUNCTION</>.
+ Marking a function <literal>CALLED ON NULL INPUT</> also requires
+ permission to use the function's language when creating new functions.
To change a function's schema, you must also have <literal>CREATE</>
privilege on the new schema.
To alter the owner, you must also be a direct or indirect member of the new
diff --git a/src/backend/commands/functioncindex 13e30f4..b40bcdf 100644
*** a/src/backend/commands/functioncmds.c
--- b/src/backend/commands/functioncmds.c
***************
*** 70,75 **** static void AlterFunctionOwner_internal(Relation rel, HeapTuple
tup,
--- 70,107 ----
/*
+ * May the current user create functions of a given language?
+ */
+ static void
+ check_language_usage(HeapTuple languageTuple)
+ {
+ Oid languageOid;
+ Form_pg_language languageStruct;
+
+ languageOid = HeapTupleGetOid(languageTuple);
+ languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
+
+ if (languageStruct->lanpltrusted)
+ {
+ /* if trusted language, need USAGE privilege */
+ AclResult aclresult;
+
+ aclresult = pg_language_aclcheck(languageOid, GetUserId(),
ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_LANGUAGE,
+
NameStr(languageStruct->lanname));
+ }
+ else
+ {
+ /* if untrusted language, must be superuser */
+ if (!superuser())
+ aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_LANGUAGE,
+
NameStr(languageStruct->lanname));
+ }
+ }
+
+
+ /*
* Examine the RETURNS clause of the CREATE FUNCTION statement
* and return information about it as *prorettype_p and *returnsSet.
*
***************
*** 864,890 **** CreateFunction(CreateFunctionStmt *stmt, const char
*queryString)
(PLTemplateExists(language) ?
errhint("Use CREATE LANGUAGE to load the
language into the database.") : 0)));
languageOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
-
- if (languageStruct->lanpltrusted)
- {
- /* if trusted language, need USAGE privilege */
- AclResult aclresult;
-
- aclresult = pg_language_aclcheck(languageOid, GetUserId(),
ACL_USAGE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_LANGUAGE,
-
NameStr(languageStruct->lanname));
- }
- else
- {
- /* if untrusted language, must be superuser */
- if (!superuser())
- aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_LANGUAGE,
-
NameStr(languageStruct->lanname));
- }
-
languageValidator = languageStruct->lanvalidator;
ReleaseSysCache(languageTuple);
--- 896,905 ----
(PLTemplateExists(language) ?
errhint("Use CREATE LANGUAGE to load the
language into the database.") : 0)));
+ check_language_usage(languageTuple);
+
languageOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
languageValidator = languageStruct->lanvalidator;
ReleaseSysCache(languageTuple);
***************
*** 1312,1318 **** AlterFunction(AlterFunctionStmt *stmt)
--- 1327,1355 ----
if (volatility_item)
procForm->provolatile =
interpret_func_volatility(volatility_item);
if (strict_item)
+ {
+ /*
+ * C-language functions that expect to be declared STRICT often
omit
+ * NULL argument checks, crashing if they do receive a NULL. To
+ * protect such functions against less-privileged owners,
clearing
+ * proisstrict requires the authority to define a new function
of the
+ * same language.
+ */
+ if (!intVal(strict_item->arg))
+ {
+ HeapTuple langTuple;
+
+ langTuple = SearchSysCache1(LANGOID,
+
PointerGetDatum(procForm->prolang));
+ if (!HeapTupleIsValid(langTuple))
+ elog(ERROR, "cache lookup failed for language
%u",
+ procForm->prolang);
+ check_language_usage(langTuple);
+ ReleaseSysCache(langTuple);
+ }
+
procForm->proisstrict = intVal(strict_item->arg);
+ }
if (security_def_item)
procForm->prosecdef = intVal(security_def_item->arg);
if (leakproof_item)
***************
*** 1974,2002 **** ExecuteDoStmt(DoStmt *stmt)
(PLTemplateExists(language) ?
errhint("Use CREATE LANGUAGE to load the
language into the database.") : 0)));
codeblock->langOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
codeblock->langIsTrusted = languageStruct->lanpltrusted;
- if (languageStruct->lanpltrusted)
- {
- /* if trusted language, need USAGE privilege */
- AclResult aclresult;
-
- aclresult = pg_language_aclcheck(codeblock->langOid,
GetUserId(),
-
ACL_USAGE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_LANGUAGE,
-
NameStr(languageStruct->lanname));
- }
- else
- {
- /* if untrusted language, must be superuser */
- if (!superuser())
- aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_LANGUAGE,
-
NameStr(languageStruct->lanname));
- }
-
/* get the handler function's OID */
laninline = languageStruct->laninline;
if (!OidIsValid(laninline))
--- 2011,2022 ----
(PLTemplateExists(language) ?
errhint("Use CREATE LANGUAGE to load the
language into the database.") : 0)));
+ check_language_usage(languageTuple);
+
codeblock->langOid = HeapTupleGetOid(languageTuple);
languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
codeblock->langIsTrusted = languageStruct->lanpltrusted;
/* get the handler function's OID */
laninline = languageStruct->laninline;
if (!OidIsValid(laninline))
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
