Re: Re: Missing REVOKE in SQL for functions with SECURITY DEFINER (or any function, really) - Mailing list pgadmin-hackers

Thanks for the prompt fix this time around!
I'll be sure to have a closer look as soon as the new version is out.
And more when I find time.

Regards
Erwin

On 22.04.2013 12:33, Dave Page wrote:
> Thanks - seems to work well, so I've committed the patch to 1.16 and master.
>
> Erwin; would appreciate any additional QA you can find time for :-)
>
> On Fri, Apr 19, 2013 at 1:47 PM, Dhiraj Chawla
> <dhiraj.chawla@enterprisedb.com> wrote:
>> Hi Dave,
>>
>> Please find attached the patch that fixes this issue related to security
>> definer not provided.
>>
>> According to our discussion based on
>> http://www.postgresql.org/docs/9.2/static/sql-grant.html the fix that I have
>> made is as follows:
>>
>> When a user creates a new function, database or language, we show their
>> default privileges in the dialog box, thus giving the user a chance to
>> revoke them if they wish to. If the user chooses to revoke them, then in the
>> reverse engineered code in the sql pane, the revoke statement will be
>> visible. For this I have modified the GetGrant function of pgObject to
>> handle this.
>>
>> Thus we ourselves don't revoke the default grant given to function, database
>> or language object, but we make sure that they are visible to the user, so
>> that he/she can take an informed decision.
>>
>> In this patch I have also taken care that if the owner of an object revokes
>> all privileges from himself, then that should also be seen in the reverse
>> engineered sql in the sql pane.
>>
>> Let me know your views on this patch.
>>
>> Thanks,
>>
>>
>>
>> On Mon, Apr 8, 2013 at 5:08 PM, Dave Page <dpage@pgadmin.org> wrote:
>>> Dhiraj, can you look into this please?
>>>
>>> Thanks.
>>>
>>> On Sun, Apr 7, 2013 at 11:02 PM, Erwin Brandstetter
>>> <brandstetter@falter.at> wrote:
>>>> 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 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
>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>
>>
>>
>> --
>> regards,
>>
>> Dhiraj Chawla
>> Software Engineer
>> EnterpriseDB Corporation
>> The Enterprise PostgreSQL Company
>>
>> Phone: +91-20-30589522
>
>