Thanks a lot Dave :-)
On Mon, Apr 22, 2013 at 4:03 PM, Dave Page <dp...@pgadmin.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.cha...@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 <dp...@pgadmin.org> wrote: > >> > >> Dhiraj, can you look into this please? > >> > >> Thanks. > >> > >> On Sun, Apr 7, 2013 at 11:02 PM, Erwin Brandstetter > >> <brandstet...@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 > > > > -- > 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
