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

Noah Misch <noah@leadboat.com> writes:
> 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();

Huh, interesting.  I coulda sworn I tried exactly that case a couple
days ago, but I must have done something different.

> However, that test hit a SIGSEGV with stack corruption:

It's not so much that the stack is corrupted as that it recurses until
the stack overflows.  fmgr_info_cxt_security sees that f() is of a
non-builtin language, so it calls fmgr_info_other_lang, which calls
fmgr_info for the call handler, which goes back to
fmgr_info_cxt_security, which sees that the call handler has SET
options, so it opts to use fmgr_security_definer for invoking the
call handler.  But back at fmgr_info_other_lang, we expect fn_addr
to be pointing directly at the call handler.  So instead of running
the call handler, we run fmgr_security_definer.  It thinks that
the function it's supposed to call is identified by
fcinfo->flinfo->fn_oid, ie the original function f() not the call
handler, so it now tries to call that function, and starts the
whole lookup process over again.  Lather, rinse, repeat.

So this seems to be a shortcoming in the fmgr.c stuff: it's not really
prepared for the possibility that a PL handler function has got any
of the attributes that would trigger use of fmgr_security_definer.

I think the most expedient fix for this would be to just ignore those
attributes, by having fmgr_info_other_lang invoke fmgr_info_cxt_security
with ignore_security = true while looking up the PL handler.  We could
alternatively add a test to see if we got back a pointer to
fmgr_security_definer, but that would require doing a function pointer
comparison which I believe to not be very reliable.

So this is a security issue after all, to the extent that you can crash
the server this way --- there's definitely no possibility of doing
something else, since the recursion-to-overflow is certain.
        regards, tom lane