Re: Not quite a security hole: CREATE LANGUAGE for non-superusers

On Wed, May 30, 2012 at 12:02:06PM -0400, Tom Lane wrote:
> 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.

I wondered about ALTER FUNCTION SET guc = '...' and tried to test it:

CREATE FUNCTION f(out ret text) RETURNS text LANGUAGE plpgsql AS
'BEGIN ret := current_setting(''work_mem''); END';
ALTER FUNCTION plpgsql_call_handler() SET work_mem = '2MB';
SELECT f();

However, that test hit a SIGSEGV with stack corruption:

#0 DirectFunctionCall1Coll (func=0x477004 <hashoid>, collation=0, arg1=65551) at fmgr.c:1018
#1 0x00000000007246c5 in CatalogCacheComputeHashValue (cache=0xc61398, nkeys=<value optimized out>, cur_skey=0x7fff026ed390) at catcache.c:210
#2 0x00000000007257c6 in SearchCatCache (cache=0xc61398, v1=65551, v2=0, v3=0, v4=0) at catcache.c:1091
#3 0x00000000007304e2 in SearchSysCache (cacheId=0, key1=65551, key2=0, key3=0, key4=0) at syscache.c:859
#4 0x000000000073d382 in fmgr_info_cxt_security (functionId=0, finfo=0x7fff026ed650, mcxt=<value optimized out>, ignore_security=0 '\000') at fmgr.c:214
#5 0x000000000073d939 in fmgr_info_cxt (functionId=4681732, finfo=0x0, mcxt=0x1000f) at fmgr.c:171
#6 0x000000000073d94e in fmgr_info (functionId=4681732, finfo=0x0) at fmgr.c:161
#7 0x000000000073d890 in fmgr_info_other_lang (functionId=65555, finfo=0x7fcc68bd54d8, mcxt=<value optimized out>, ignore_security=<value optimized out>) at fmgr.c:408
#8 fmgr_info_cxt_security (functionId=65555, finfo=0x7fcc68bd54d8, mcxt=<value optimized out>, ignore_security=<value optimized out>) at fmgr.c:290
#9 0x000000000073e8dd in fmgr_security_definer (fcinfo=0xccae20) at fmgr.c:901
#10 0x000000000073eb24 in fmgr_security_definer (fcinfo=0x477004) at fmgr.c:968
#11 0x000000000073eb24 in fmgr_security_definer (fcinfo=0x477004) at fmgr.c:968
...
#2526 0x000000000073eb24 in fmgr_security_definer (fcinfo=0x477004) at fmgr.c:968
...

> 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?

Stepping outside the special case of language support functions, it follows
that 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. (The argument types for procedural language support functions
keep this from affecting them directly.)

> 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.

Sounds like a clear improvement for CREATE LANGUAGE, seeing those functions go
into pg_catalog. I'm less sure about CREATE RANGE TYPE, but I cannot think of
a concrete reason not to do so.

Thanks,
nm