Re: [HACKERS] pg_ls_dir & friends still have a hard-coded superuser check

On Thu, Jan 26, 2017 at 5:36 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> The first is that restricting the ability to GRANT access
>> to a function, even a function that allows escalation to superuser
>> privileges, doesn't improve security, because the superuser can still
>> grant those privileges with more work.
>
> Having various inconsistent and unclear ways to grant access to a
> privileged user is making security worse and that is very much part of
> my concern. I don't agree, regardless of how many times you claim it,
> that because the superuser could still grant superuser to someone (or
> could grant the individual privileges with more work) keeps things
> status-quo regarding security, or somehow that not making the changes
> you are proposing reduces existing security.

I think it certainly increases security, especially when the functions
don't themselves confer superuser access, because then people won't
use superuser access when something less will do.  I agree that
pg_read_file() will often allow an escalation to superuser, but not
always, and not necessarily easily.  If somebody breaks into the
account I'm using for monitoring my system, I'd way, way rather have
them get pg_read_file() than superuser.  And for pg_ls_dir(), about a
hundred times moreso.

> Why aren't you including the other functions mentioned?  Not including
> them, even if they're in contrib, would still end up with things in an
> inconssitent state, and it hardly seems like it'd be much effort to go
> through the rest of them with equal care.

The contrib stuff is harder to change because of
backward-compatibility with previous extensions.  I could go through
and analyze it if we had some consensus on the basic principle here,
but we don't seem to agree on anything.  Your current policy proposal,
as I understand it, is that things should have builtin superuser
privileges if (a) there is any chance they can be used to escalate to
superuser or (b) they involve any access to the filesystem, even
access that can't be used to escalate to superuser.  Even if I were to
agree to (a), (b) looks like a random wart to justify the status quo,
so I'm not real hopeful about further analysis finding any common
ground.  But that having been said, the broad picture here is that
there are two contrib modules which have hard-coded superuser checks
at the top of SQL-callable functions:

- contrib/adminpack has some very dangerous functions (pg_file_write,
pg_file_rename, pg_file_unlink) that have a hard-coded superuser
check.  It also has an apparently-not-that-dangerous function
(pg_logdir_ls) with such a check.

- contrib/pageinspect has lots of superuser checks, basically because
they have known input-validation weaknesses.  See
3e1338475ffc2eac25de60a9de9ce689b763aced for the rationale in detail.
The purist in me is inclined to think that the best answer here would
be to fix the functions so that they're safe, but that might not be
terribly easy to do.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company