ALTER ROLE/DATABASE RESET ALL versus security

It looks to me like the code in AlterSetting() will allow an ordinary
user to blow away all settings for himself. Even those that are for
SUSET variables and were presumably set for him by a superuser. Isn't
this a security hole? I would expect that an unprivileged user should
not be able to change such settings, not even to the extent of
reverting to the installation-wide default.

regards, tom lane

--On 13. November 2009 19:08:22 -0500 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> It looks to me like the code in AlterSetting() will allow an ordinary
> user to blow away all settings for himself. Even those that are for
> SUSET variables and were presumably set for him by a superuser. Isn't
> this a security hole? I would expect that an unprivileged user should
> not be able to change such settings, not even to the extent of
> reverting to the installation-wide default.

I agree. A quick check shows that resetting or changing a single parameter
is not allowed, so this seems inconsistent anyways. Maybe AlterSetting()
should be more strict and pick only those settings, which are safe for
ordinary users to reset?

--
Thanks

Bernd

Tom Lane wrote:
> It looks to me like the code in AlterSetting() will allow an ordinary
> user to blow away all settings for himself. Even those that are for
> SUSET variables and were presumably set for him by a superuser. Isn't
> this a security hole? I would expect that an unprivileged user should
> not be able to change such settings, not even to the extent of
> reverting to the installation-wide default.

Yes, I completely overlooked the fact that users should not be able to
blow away GUCs set by superuser. I can't handle this right now though,
as I'm leaving in a couple of days and won't return until cca. Dec. 1st.
If this can wait (and I think it does) then I'll handle it then;
otherwise I'd appreciate if someone else could take a look and fix it.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Tom Lane wrote:
> It looks to me like the code in AlterSetting() will allow an ordinary
> user to blow away all settings for himself. Even those that are for
> SUSET variables and were presumably set for him by a superuser. Isn't
> this a security hole? I would expect that an unprivileged user should
> not be able to change such settings, not even to the extent of
> reverting to the installation-wide default.

Yes, it is, but this is not a new hole. This works just fine in 8.4
too:

alvherre=# create role foo;
CREATE ROLE
alvherre=# alter role foo set lc_messages = 'C';
ALTER ROLE
alvherre=# set session AUTHORIZATION foo;
SET
alvherre=> show lc_messages ;
lc_messages
-------------
es_CL.UTF-8
(1 fila)

alvherre=> alter role foo reset all;
ALTER ROLE
alvherre=> reset session AUTHORIZATION ;
RESET
alvherre=# set session AUTHORIZATION foo;
SET
alvherre=> show lc_messages ;
lc_messages
-------------
es_CL.UTF-8
(1 fila)

alvherre=> alter role foo set lc_messages to 'C';
ERROR: se ha denegado el permiso para cambiar la opción «lc_messages»

So any user is able to reset settings that were set for him by the
superuser.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tom Lane wrote:
>> It looks to me like the code in AlterSetting() will allow an ordinary
>> user to blow away all settings for himself. Even those that are for
>> SUSET variables and were presumably set for him by a superuser. Isn't
>> this a security hole? I would expect that an unprivileged user should
>> not be able to change such settings, not even to the extent of
>> reverting to the installation-wide default.

> Yes, it is, but this is not a new hole. This works just fine in 8.4
> too:

So I'd argue for changing it in 8.4 too.

regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > Tom Lane wrote:
> >> It looks to me like the code in AlterSetting() will allow an ordinary
> >> user to blow away all settings for himself. Even those that are for
> >> SUSET variables and were presumably set for him by a superuser. Isn't
> >> this a security hole? I would expect that an unprivileged user should
> >> not be able to change such settings, not even to the extent of
> >> reverting to the installation-wide default.
>
> > Yes, it is, but this is not a new hole. This works just fine in 8.4
> > too:
>
> So I'd argue for changing it in 8.4 too.

Understood. I'm starting to look at what this requires.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:
> Tom Lane wrote:
> > Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > > Tom Lane wrote:
> > >> It looks to me like the code in AlterSetting() will allow an ordinary
> > >> user to blow away all settings for himself. Even those that are for
> > >> SUSET variables and were presumably set for him by a superuser. Isn't
> > >> this a security hole? I would expect that an unprivileged user should
> > >> not be able to change such settings, not even to the extent of
> > >> reverting to the installation-wide default.
> >
> > > Yes, it is, but this is not a new hole. This works just fine in 8.4
> > > too:
> >
> > So I'd argue for changing it in 8.4 too.
>
> Understood. I'm starting to look at what this requires.

Any progress on this?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Alvaro Herrera wrote:
> > Tom Lane wrote:
> > > Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > > > Tom Lane wrote:
> > > >> It looks to me like the code in AlterSetting() will allow an ordinary
> > > >> user to blow away all settings for himself. Even those that are for
> > > >> SUSET variables and were presumably set for him by a superuser. Isn't
> > > >> this a security hole? I would expect that an unprivileged user should
> > > >> not be able to change such settings, not even to the extent of
> > > >> reverting to the installation-wide default.
> > >
> > > > Yes, it is, but this is not a new hole. This works just fine in 8.4
> > > > too:
> > >
> > > So I'd argue for changing it in 8.4 too.
> >
> > Understood. I'm starting to look at what this requires.
>
> Any progress on this?

I have come up with the attached patch. I haven't tested it fully yet,
and I need to backport it. The gist of it is: we can't simply remove
the pg_db_role_setting tuple, we need to ask GUC to reset the settings
array, for which it checks superuser-ness on each setting.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> I have come up with the attached patch. I haven't tested it fully yet,
> and I need to backport it. The gist of it is: we can't simply remove
> the pg_db_role_setting tuple, we need to ask GUC to reset the settings
> array, for which it checks superuser-ness on each setting.

I think you still want to have a code path whereby the tuple will be
deleted once the array is empty. Failing to check that is inefficient
and also exposes clients such as pg_dump to corner case bugs.

regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > I have come up with the attached patch. I haven't tested it fully yet,
> > and I need to backport it. The gist of it is: we can't simply remove
> > the pg_db_role_setting tuple, we need to ask GUC to reset the settings
> > array, for which it checks superuser-ness on each setting.
>
> I think you still want to have a code path whereby the tuple will be
> deleted once the array is empty. Failing to check that is inefficient
> and also exposes clients such as pg_dump to corner case bugs.

Yeah, that's there too -- it behaves the same way as ALTER / RESET for a
particular setting.

I just applied it all the way back to 7.4. It was a bit of a pain to
backport it, because every version seemed to have this or that little
incompatibility.

I attempted a regression test, but it's also painful because there's no
nice way to clean up after a newly created user (i.e. drop it): after
the last \c - regress_user_guc, there's no way to go back to the
original user. And we can't use SET SESSION AUTHORIZATION because it
doesn't cause the settings for the role to be loaded. (I think that's a
bug too). Suggestions on how to enable this are welcome.

-- Test user-specific settings
create role regress_user_guc login;
alter role regress_user_guc set work_mem to '128MB';
alter role regress_user_guc set lc_messages to 'C';
\c - regress_user_guc
select name, setting, source
from pg_settings
where name in ('work_mem', 'lc_messages')
order by name;
alter role regress_user_guc reset all;
\c - regress_user_guc
-- can't display actual value here because it may be installation-dependant
select name, setting, source
from pg_settings
where name in ('work_mem', 'lc_messages')
order by name;

(I think I should also use a superuser setting other than lc_messages).

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.