Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers) - Mailing list pgsql-hackers

From Noah Misch
Subject Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)
Date
Msg-id 20120611171920.GF10817@tornado.leadboat.com
Whole thread Raw
In response to Re: Not quite a security hole: CREATE LANGUAGE for non-superusers  (Noah Misch <noah@leadboat.com>)
Responses Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Dave Page
Date:
Subject: Re: 9.2 final
Next
From: Alex
Date:
Subject: Re: Inconsistency in libpq connection parameters, and extension thereof