Re: Missing REVOKE in SQL for functions with SECURITY DEFINER (or any function, really)

Hi developers!

I have been missing in action for a while, so I am not sure whether
anybody even uses trac any more.
Either way, I just ran into this bug once again and checked to find it
still open:
http://code.pgadmin.org/trac/ticket/88

Basically, REVOKE EXECUTE ON FUNCTION is omitted in the DDL script.
To reproduce:

CREATE OR REPLACE FUNCTION foo() RETURNS int AS 'SELECT 1' LANGUAGE sql;
REVOKE EXECUTE ON FUNCTION foo() FROM public;

This is a **potential security hazard** and it has been open for (at
least) over a year now.

Regards
Erwin

On 27.02.2012 23:53, Erwin Brandstetter wrote:
> On 27.02.2012 23:38, Erwin Brandstetter wrote:
>> Hi developers!
>>
>> Congratulations on the many bug fixes in the latest release!
>> I think I found another serious problem.
>>
>> Testing with pgAdmin 1.14.2 on Windows XP. Server is PostgreSQL 9.1
>> on Devian Squeeze.
>>
>> There is a security hazard lingering in the reverse engineered SQL of
>> the latest version 1.14.2 (and versions before it).
>>
>> As summed up here
>> http://www.postgresql.org/docs/current/interactive/sql-createfunction.html#SQL-CREATEFUNCTION-SECURITY
>> the execute privilege is granted to PUBLIC by default. It needs to be
>> revoked for security critical functions.
>>
>> I quote the manual:
>>> Another point to keep in mind is that by default, execute privilege
>>> is granted to PUBLIC for newly created functions (see GRANT
>>> <http://www.postgresql.org/docs/current/interactive/sql-grant.html>
>>> for more information). Frequently you will wish to restrict use of a
>>> security definer function to only some users. To do that, you must
>>> revoke the default PUBLIC privileges and then grant execute
>>> privilege selectively.
>>
>> This goes wrong with pgAdmin 1.14.2. Consider this test case,
>> executed as superuser postgres:
>>
>> CREATE OR REPLACE FUNCTION foo ()
>> RETURNS void AS
>> $BODY$
>> BEGIN
>> PERFORM 1;
>> END;
>> $BODY$
>> LANGUAGE plpgsql VOLATILE SECURITY DEFINER;
>> ALTER FUNCTION foo() SET search_path=public, pg_temp;
>> *REVOKE ALL ON FUNCTION foo() FROM PUBLIC;*
>> GRANT EXECUTE ON FUNCTION foo() TO ief;
>>
>>
>> The reverse engineered SQL looks like this
>>
>> -- Function: foo()
>>
>> -- DROP FUNCTION foo();
>>
>> CREATE OR REPLACE FUNCTION foo()
>> RETURNS void AS
>> $BODY$
>>
>> BEGIN
>> PERFORM 1;
>> END;
>> $BODY$
>> LANGUAGE plpgsql VOLATILE SECURITY DEFINER
>> COST 100;
>> ALTER FUNCTION foo() SET search_path=public, pg_temp;
>>
>> ALTER FUNCTION foo()
>> OWNER TO postgres;
>> GRANT EXECUTE ON FUNCTION foo() TO postgres;
>> GRANT EXECUTE ON FUNCTION foo() TO ief;
>>
>>
>> The REVOKE statement is missing, which is a serious security hazard.
>> A recreated function will be open to the the public.
>>
>> Regards
>> Erwin
>
> I reopened ticket #88 for that
> http://code.pgadmin.org/trac/ticket/88#comment:2
> because it seemed closely related.
>
> Regards
> Erwin