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

Thanks a lot Dave :-)

On Mon, Apr 22, 2013 at 4:03 PM, Dave Page <dpage(at)pgadmin(dot)org> 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(dot)chawla(at)enterprisedb(dot)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(at)pgadmin(dot)org> wrote:
> >>
> >> Dhiraj, can you look into this please?
> >>
> >> Thanks.
> >>
> >> On Sun, Apr 7, 2013 at 11:02 PM, Erwin Brandstetter
> >> <brandstetter(at)falter(dot)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
>
>
>
> --
> 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